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

Jakub Hrozek jhrozek at redhat.com
Fri Nov 28 15:12:40 UTC 2014


On Thu, Nov 27, 2014 at 03:59:38PM +0100, Pavel Reichl wrote:
> 
> On 11/24/2014 10:43 PM, Michal Židek wrote:
> >On 11/24/2014 10:08 PM, Lukas Slebodnik wrote:
> >>On (24/11/14 19:29), Jakub Hrozek wrote:
> >>>On Mon, Nov 24, 2014 at 06:37:04PM +0100, Lukas Slebodnik wrote:
> >>>>On (24/11/14 17:21), Michal Židek wrote:
> >>>>>On 11/24/2014 11:16 AM, Pavel Reichl wrote:
> >>>>>>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?
> >>>>>
> >>>>>I did to avoid calling discard_const_p. I also think the code
> >>>>>is easier to follow if the opt_override is used instead of
> >>>>>string literrals.
> >>>>>
> >>>>NACK
> >>>>
> >>>>4th argument is not modified in confdb_set_string thus API can be
> >>>>changed
> >>>
> >>>True, but also completely unrelated to this patch.
> >>That's not really true.
> >>
> >>Michal wanted to avoid to using discard_const_p, therefore he called
> >>talloc_strdup which is overkill in this case.
> >>
> >>LS
> >
> >I agree with Lukas. I was not thinking clearly and got myself
> >into some 'discard const warning disposal' trap. New patches
> >attached.
> >
> >Michal
> >
> >
> >
> >_______________________________________________
> >sssd-devel mailing list
> >sssd-devel at lists.fedorahosted.org
> >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> ACK

* master:
    5b4c6f22cb576a11037c7fa940fe0ba09e643e77
    4b6fa94d1a00b07c4310678ec721282288a186a0



More information about the sssd-devel mailing list