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

Michal Židek mzidek at redhat.com
Mon Nov 24 21:43:35 UTC 2014


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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-confdb-Make-confdb_set_string-accept-const-char-poin.patch
Type: text/x-patch
Size: 1417 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20141124/ea881a96/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-AD-Never-store-case_sensitive-as-true-to-confdb.patch
Type: text/x-patch
Size: 1989 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20141124/ea881a96/attachment-0001.bin>


More information about the sssd-devel mailing list