On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
> On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
>> On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:
>>> On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:
>>>> On (23/06/16 18:59), Michal Židek wrote:
>>>>> On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:
>>>>>> On (23/06/16 18:24), Michal Židek wrote:
>>>>>>> On 06/23/2016 05:50 PM, Michal Židek wrote:
>>>>>>>> Lukas,
>>>>>>>>
>>>>>>>> did you have some local patches related to
>>>>>>>> this patch, that you did not send here?
>>>>>>>>
>>>>>>>> Because I can not apply the patch on current
>>>>>>>> master and the changes in the context of
>>>>>>>> your patch seem like you did something with
>>>>>>>> default config file path?
>>>>>>>>
>>>>>>>> Slightly modified version that applies on
>>>>>>>> current master is attached (the changes
>>>>>>>> are the same, but the context differs).
>>>>>>>>
>>>>>>>> Michal
>>>>>>>>
>>>>>>>
>>>>>>> I added the missing access check for
>>>>>>> snippets. If the snippets does not
>>>>>>> pass the access check it is skipped
>>>>>>> and a loud debug message is logged.
>>>>>>>
>>>>>>> I attach it as separate patch for
>>>>>>> easier review, but it should be squashed
>>>>>>> before pushing. I also attach the first
>>>>>>> patch for convenience.
>>>>>>>
>>>>>>> Michal
>>>>>>
>>>>>>> From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17
00:00:00 2001
>>>>>>> From: =?UTF-8?q?Michal=20=C5=BDidek?=
<mzidek(a)redhat.com>
>>>>>>> Date: Tue, 22 Mar 2016 14:09:34 +0100
>>>>>>> Subject: [PATCH] confdb: Make it possible to use config
snippets
>>>>>>>
>>>>>>> Resolves:
>>>>>>>
https://fedorahosted.org/sssd/ticket/2247
>>>>>>>
>>>>>>> Signed-off-by: Lukas Slebodnik <lslebodn(a)redhat.com>
>>>>>>> ---
>>>> LGTM.
>>>>
>>>> It isn't ACK because I was involved in development
>>>> and I didn;t test last version but code is fine fore me.
>>>
>>> I just ran a quick test and the patch works for me and the code reads OK
>>> as well. I will push after CI finishes.
>>>
>>> I also opened:
>>>
https://fedorahosted.org/sssd/ticket/3062
>>> because the patch is missing documentation and
>>>
https://fedorahosted.org/sssd/ticket/3063
>>> to add at least a simple integration test.
>>
>> Actually, can we get rid of this warning?
>>
>> Error: COMPILER_WARNING:
>> sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable
'patterns' [-Wunused-variable]
>> # const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
>> # ^
>> # 214| int ret;
>> # 215| #ifdef HAVE_LIBINI_CONFIG_V1
>> # 216|-> const char *patterns[] = { "^[^\\.].*\\.conf", NULL
};
>> # 217| const char *sections[] = { ".*", NULL };
>> # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
>>
>> Error: COMPILER_WARNING:
>> sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function
'sss_ini_get_config'
>> sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable
'sections' [-Wunused-variable]
>> # const char *sections[] = { ".*", NULL };
>> # ^
>> # 215| #ifdef HAVE_LIBINI_CONFIG_V1
>> # 216| const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
>> # 217|-> const char *sections[] = { ".*", NULL };
>> # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
>> # 219| uint32_t i = 0;
>> _______________________________________________
>
> New patch attached.
>
> Michal
>
The patch is fine, (so ack also from me), but can someone check if the
attached rebase on top of Pavel's sssctl is OK?
I did not check the differences between the rebased and
non-rebased version, but I just tried branch with sssctl
patches + this rebased patch + startup validation patches
and it worked for me.
But the CI failed for me with the snippets in place.
Will take a look at what is happening there.
Michal