[SSSD] [PATCHES] AD: support for AD site override

Jakub Hrozek jhrozek at redhat.com
Mon Jan 12 20:21:15 UTC 2015


On Fri, Jan 09, 2015 at 04:56:46PM +0100, 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.
> >
> >
> >_______________________________________________
> >sssd-devel mailing list
> >sssd-devel at lists.fedorahosted.org
> >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> Thank for comments. Amended patches attached. Thanks!

Thank you, I amended the design page and linked it with the ticket.



More information about the sssd-devel mailing list