On Wednesday, 14 January 2026 09:01:49 Central European Standard Time Stefan
Metzmacher wrote:
> Hi Andreas,
Hi Metze,
> > diff --git a/source3/libads/kerberos.c b/source3/libads/kerberos.c
> > index 5593364c397..ed8d52f7866 100644
> > --- a/source3/libads/kerberos.c
> > +++ b/source3/libads/kerberos.c
> > @@ -1259,10 +1259,10 @@ static char *get_kdc_ip_string(char *mem_ctx,
> >
> > cldap_reply = &responses[i]->data.nt5_ex;
> >
> > if (cldap_reply->pdc_dns_name != NULL) {
> >
> > - status = check_negative_conn_cache(
> > + bool has_entry = has_negative_conn_cache_entry(
> >
> > realm,
> > cldap_reply->pdc_dns_name);
> >
> > - if (!NT_STATUS_IS_OK(status)) {
> > + if (has_entry) {
> >
> > /* propagate blacklisting from name to ip */
> > add_failed_connection_entry(realm,
addr,add_failed_connection_entry( status);
> > continue;
>
> This looks strange, what is 'status' at this point?
> It used to be the status stored in the name based record.
>
> I'm wondering about the motivation about this patch,
> my first reaction was: this is not simpler now.
In libads/ldap.c we set it to NT_STATUS_UNSUCCESSFUL in case for the IP
address. We could just do the same here.
If you have a function check_negative_cache_conn and it return NT_STATUS_OK
and reports that in the log. What does it mean than. That the connection is in
the negative connection cache or not.
Nobody understands the log message without knowing the code.
Also in 99% of the cases we do not care about the result returned. The issue
you showed is the only time we use the return value to hand it to
add_failed_connection_entry().
--
Andreas Schneider [email protected]
Samba Team www.samba.org
GPG-ID: 8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D
