[SSSD] [PATCH] AD: Never store case_sensitive as "true" to confdb

Pavel Reichl preichl at redhat.com
Mon Nov 24 10:16:25 UTC 2014


Hello Michal, please see my comment inline.

On 11/21/2014 10:34 PM, Michal Židek wrote:
>
>  From 4911463b9ef5ce6276bc52dafa0ed61381ff329d Mon Sep 17 00:00:00 2001
> From: Michal Zidek<mzidek at redhat.com>
> Date: Fri, 21 Nov 2014 20:12:11 +0100
> Subject: [PATCH 2/2] AD: Never store case_sensitive as "true" to confdb
>
> If case_sensitive was set 'true' for AD
> backend, we ignore it and continue with AD
> default (false). However we still set confdb
> to whatever was set in sssd.conf for the
> responders. We should store to confdb
> the value that is used by the backend.
>
> Also fixes some misleading DEBUG messages
> in that code area.
> ---
>   src/providers/ad/ad_common.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
> index 7b08c2b..c9c7931 100644
> --- a/src/providers/ad/ad_common.c
> +++ b/src/providers/ad/ad_common.c
> @@ -264,6 +264,7 @@ ad_get_common_options(TALLOC_CTX *mem_ctx,
>       char *ad_hostname;
>       char hostname[HOST_NAME_MAX + 1];
>       char *case_sensitive_opt;
> +    char *opt_override;
>   
>       opts = talloc_zero(mem_ctx, struct ad_options);
>       if (!opts) return ENOMEM;
> @@ -359,20 +360,26 @@ ad_get_common_options(TALLOC_CTX *mem_ctx,
>           goto done;
>       }
>   
> +    opt_override = talloc_strdup(mem_ctx,
> +                                 dom->case_preserve ? "preserving" : "false");
Do we really have to allocate memory on heap?
And if we really do, I would follow this quote of Stephen:

> We should stick with the pattern of *never* allocating memory that isn't
> being returned to the parent on the parent context. It is *very*
> difficult to catch leaks that way (particularly if the parent context
> ends up being very long-lived)

> +    if (opt_override == NULL) {
> +        ret = ENOMEM;
> +        goto done;
> +    }
> +
>       /* Set this in the confdb so that the responders pick it
>        * up when they start up.
>        */
> -    ret = confdb_set_string(cdb, conf_path, "case_sensitive",
> -                            case_sensitive_opt);
> +    ret = confdb_set_string(cdb, conf_path, "case_sensitive", opt_override);
>       if (ret != EOK) {
>           DEBUG(SSSDBG_CRIT_FAILURE,
> -              "Could not set domain case-sensitive: [%s]\n",
> +              "Could not set domain option case_sensitive: [%s]\n",
>                  strerror(ret));
>           goto done;
>       }
>   
>       DEBUG(SSSDBG_CONF_SETTINGS,
> -          "Setting domain case-insensitive\n");
> +          "Setting domain option case_sensitive to [%s]\n", opt_override);
>   
>       ret = EOK;
>       *_opts = opts;
> -- 1.9.3

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20141124/15d8b610/attachment.html>


More information about the sssd-devel mailing list