[SSSD] [PATCH] DB: Switch to new libini_config API

Jakub Hrozek jhrozek at redhat.com
Fri Apr 19 11:29:27 UTC 2013


On Fri, Apr 19, 2013 at 12:37:41PM +0200, Lukas Slebodnik wrote:
> On (11/04/13 11:52), Ondrej Kos wrote:
> >On 04/11/2013 09:02 AM, Lukas Slebodnik wrote:
> >>On (10/04/13 13:54), Dmitri Pal wrote:
> >>>On 04/10/2013 10:17 AM, Ondrej Kos wrote:
> >>>>On 03/29/2013 01:19 PM, Jakub Hrozek wrote:
> >>>>>On Thu, Mar 28, 2013 at 03:59:25PM +0100, Ondrej Kos wrote:
> >>>>>>Hi,
> >>>>>>
> >>>>>>Attached is patch for https://fedorahosted.org/sssd/ticket/1786
> >>>>>>
> >>>>>>Patch extends sssd code so it's capable to build with current
> >>>>>>version of libini_config (0.7 at the time, supported versions up
> >>>>>>from 0.6.1) and with version 1.0.0, which will be released soon.
> >>>>>>
> >>>>>
> >>>>>I can use this patch as an interim solution in rawhide and F19 until we
> >>>>>ack it but it needs some work.
> >>>>>
> >>>>>Mainly, please don't use ifdefs in general code. ifdefs should be
> >>>>>used in
> >>>>>a wrapper module to provide kind of generic methods the user would then
> >>>>>call but ifdefs in main code obfuscate it. See src/util/sss_krb5.c or
> >>>>>src/util/sss_ldap.c for examples.
> >>>>>
> >>>>>We should amend our coding guidelines to specifically advise to put
> >>>>>ifdefs into a wrapper module.
> >>>>>
> >>>>>I know that there is the netlink module which does the same but arguably
> >>>>>the netlink module is a wrapper module mostly and actually we might want
> >>>>>to split it in future (feel free to file a ticket).
> >>>>>
> >>>>>So there should be a src/util/sss_col.c module or something like that
> >>>>>that would export functions or macros with the same prototype no matter
> >>>>>the libini version. That module will contain a function that will return
> >>>>>ldif which can be then consumed by confdb.
> >>>>>
> >>>>>Also please don't use the old debug levels in new code.
> >>>>
> >>>>Hi,
> >>>>
> >>>>New patch is attached, it supplies src/util/sss_ini.c &
> >>>>src/util/sss_ini.h, which are exporting functions used in confdb_setup.
> >>>>
> >>>>Please, let me know, if this is the approach you meant, and with
> >>>>comments for the coding.
> >>>>
> >>>
> >>>I scanned the patch and I generally like the approach.
> >>>Prefix "iniw_" looks a bit weird because "w" does not ring the bell but
> >>>this is just me. :-)
> >
> >This was leftover from previous patch, it was inspired from when I
> >was working on the libnl support. iniw stands for ini wrapper, I
> >changed it to respect style of the rest of the module.
> >
> >>>
> >>>
> >>>>Ondra
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>_______________________________________________
> >>>>sssd-devel mailing list
> >>>>sssd-devel at lists.fedorahosted.org
> >>>>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >>>
> >>>
> >>>--
> >>>Thank you,
> >>>Dmitri Pal
> >>>
> >>>Sr. Engineering Manager for IdM portfolio
> >>>Red Hat Inc.
> >>
> >>I tried just ot compile with both version of libini.
> >>
> >>$ make distcheck
> >>
> >>...
> >>
> >>   CC src/util/sss_ini.lo
> >>   ../src/util/sss_ini.c:31:26: fatal error: util/sss_ini.h: No such file or directory
> >>   compilation terminated.
> >>   make[3]: *** [src/util/sss_ini.lo] Error 1
> >>   make[3]: Leaving directory `/dev/shm/sssd_build/sssd-1.9.92/_build'
> >>   make[2]: *** [all-recursive] Error 1
> >>   make[2]: Leaving directory `/dev/shm/sssd_build/sssd-1.9.92/_build'
> >>   make[1]: *** [all] Error 2
> >>   make[1]: Leaving directory `/dev/shm/sssd_build/sssd-1.9.92/_build'
> >>   make: *** [distcheck] Error 1
> >>
> >>You forgot to add src/util/sss_ini.h to dist_noinst_HEADERS im Makafile.am
> >>
> >>LS
> >
> >Thanks, I must've missed it.
> >
> >>_______________________________________________
> >>sssd-devel mailing list
> >>sssd-devel at lists.fedorahosted.org
> >>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >>
> >
> >Updated patch attached.
> >
> >Ondra
> >
> 
> Sorry for late answer.
> 
> Your patch works fine with libini_config >=1.0.0.1 (fedora 19).
> But sssd segfault with libini_config=0.7.0 (fedora 18)
> 
> Core was generated by `sssd -i'.
> Program terminated with signal 11, Segmentation fault.
> #0  fstat (__statbuf=0xd525d8, __fd=<error reading variable: Cannot access memory at address 0x5>) at /usr/include/sys/stat.h:470
> 470       return __fxstat (_STAT_VER, __fd, __statbuf);
> #0  fstat (__statbuf=0xd525d8, __fd=<error reading variable: Cannot access memory at address 0x5>) at /usr/include/sys/stat.h:470
> #1  sss_ini_get_stat (file_desc=0x5, cstat=cstat at entry=0xd525d8) at src/util/sss_ini.c:101
> #2  0x0000000000414b73 in confdb_init_db (config_file=config_file at entry=0xd4a400 "/etc/sssd/sssd.conf", cdb=0xd4a7e0) at src/confdb/confdb_setup.c:172
> #3  0x000000000040485c in load_configuration (monitor=<synthetic pointer>, config_file=0xd4a400 "/etc/sssd/sssd.conf", mem_ctx=0xd4a390) at src/monitor/monitor.c:1613
> #4  main (argc=<optimized out>, argv=<optimized out>) at src/monitor/monitor.c:2766
> 
> LS

We also did a short review in person. tl;dr -- do not use void* but make
the structure opaque and always pass the pointer to the structure
regardless of its contents.



More information about the sssd-devel mailing list