[SSSD] [PATCHES] AD: support for AD site override
Pavel Březina
pbrezina at redhat.com
Thu Jan 15 10:21:11 UTC 2015
On 01/09/2015 04:56 PM, Pavel Reichl wrote:
>
> On 01/08/2015 08:36 PM, Pavel Březina wrote:
>> On 01/07/2015 03:28 PM, Pavel Reichl wrote:
>>> Please see attached patches.
>>>
>>> Thanks.
>>
>> Hi,
>>
>> I know there is a design page I did not comment on but seeing the
>> patch now, does the option really have to be names ad_site_override?
>> Wouldn't simple ad_site be a better name?
>>
>> I'd change the description to something like:
>> "Specify AD site client should try to connect to. If this option is
>> not set, the AD site will be auto-discovered."
>>
>> We are using talloc_zero so the following code:
>>
>>> @@ -591,6 +593,14 @@ ad_srv_plugin_ctx_init(TALLOC_CTX *mem_ctx,
>>> goto fail;
>>> }
>>>
>>> + if (ad_site_override == NULL) {
>>> + ctx->ad_site_override = NULL;
>>> + } else {
>>> + ctx->ad_site_override = talloc_strdup(ctx, ad_site_override);
>>> + if (ctx->ad_site_override == NULL) {
>>> + goto fail;
>>> + }
>>> + }
>>> return ctx;
>>
>> can be simplified to:
>>
>>> if (ad_site_override != NULL) {
>>> ctx->ad_site_override = talloc_strdup(ctx, ad_site_override);
>>> if (ctx->ad_site_override == NULL) {
>>> goto fail;
>>> }
>>> }
>> > (please add an empty line here)
>>> return ctx;
>>
>>> @@ -756,6 +766,19 @@ static void ad_srv_plugin_site_done(struct
>>> tevent_req *subreq)
>>>
>>> ret = ad_get_client_site_recv(state, subreq, &state->site,
>>> &state->forest);
>>> talloc_zfree(subreq);
>>> + /* Ignore AD site found by dns discovery if specific site is set in
>>> + * configuration file. */
>>> + 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 = talloc_strdup(state,
>>> state->ctx->ad_site_override);
>>> + if (state->site == NULL) {
>>> + ret = ENOMEM;
>>> + goto done;
>>> + }
>>> + }
>>
>> There is no need to strdup the site name if you make state->site const.
man page:
Specify AD site client should try to connect to.
Specify AD site to which client should try to connect.
Which one sounds better?
Ack otherwise.
More information about the sssd-devel
mailing list