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?