On Tue, Jul 28, 2015 at 12:13:31PM +0200, Pavel Březina wrote: > On 07/27/2015 09:35 PM, Jakub Hrozek wrote: > >On Mon, Jul 27, 2015 at 07:38:12PM +0200, Pavel Březina wrote: > >>On 07/27/2015 05:38 PM, Jakub Hrozek wrote: > >>>On Mon, Jul 27, 2015 at 12:32:59PM +0200, Pavel Březina wrote: > >>>>On 07/21/2015 09:45 PM, Jakub Hrozek wrote: > >>>>>Hi, > >>>>> > >>>>>the attached patch helps AD clients that have trouble detecting the > >>>>>right site. Please see the patch and the commit message for more > >>>>>details. > >>>> > >>>>Hmm... nack. I think this change makes better job: > >>>> > >>>> if (state->ctx->ad_site_override != NULL) { > >>>> DEBUG(SSSDBG_TRACE_INTERNAL, > >>>> "Ignoring AD site found by DNS discovery: '%s', " > >>>> "using configured value: '%s' instead.\n", > >>>> state->site, state->ctx->ad_site_override); > >>>> state->site = state->ctx->ad_site_override; > >>>>+ ret = EOK; > >>>> } > >>> > >>>That was my first version, too, but it appears that the EOK branch tries > >>>to access state->forest that is not known -- keep in mind that this is > >>>for cases where ad_get_client_site_recv() returned ENOENT so its output > >>>parameters might not be available. > >>> > >>>Can we infer the forest somehow? > >> > >>Is forest not known or is it just not set because _recv returned error? > > > >I don't think we can make any assumptions unless we change the _recv to > >return a special error code for the case where the request was only able > >to read the forest. But looking at the request, both are read at the > >same time using ad_get_client_site_parse_ndr(). > > > >>The > >>latter one can be solved... but it may be even more conditions :-) > >> > >>OK then. > > > >Can you review the patch, please? > > How about this: > > diff --git a/src/providers/ad/ad_srv.c b/src/providers/ad/ad_srv.c > index f3a4e64..dabcf9f 100644 > --- a/src/providers/ad/ad_srv.c > +++ b/src/providers/ad/ad_srv.c > @@ -774,6 +774,12 @@ static void ad_srv_plugin_site_done(struct tevent_req > *subreq) > "using configured value: '%s' instead.\n", > state->site, state->ctx->ad_site_override); > state->site = state->ctx->ad_site_override; > + > + if (state->forest == NULL) { > + state->forest = state->discovery_domain; > + } > + > + ret = EOK; > } > > With some debug message probably.
Yes, that's better, do you want to send this as a git-formatted patch? _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel