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?