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

Pavel Reichl preichl at redhat.com
Thu Nov 27 14:59:38 UTC 2014


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20141127/eb015674/attachment.html>


More information about the sssd-devel mailing list