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@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@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
ACK