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

Ondrej Kos okos at redhat.com
Fri Apr 19 14:05:49 UTC 2013


On 04/19/2013 01:29 PM, Jakub Hrozek wrote:
> 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

This was really weird, it worked correctly in my environment, even if i 
rebased on the current master, but started segfaulting when i backed few 
patches. The code was wrong, but it wasn't failing.

>
> 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.

My first idea was for the functions to accept just the necessary 
variables, but this will be better, the code is more readable now and 
the maintenance will be easier, thanks for advice!

New patch is attached

Ondra

> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>


-- 
Ondrej Kos
Associate Software Engineer
Identity Management
Red Hat Czech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-DB-Switch-to-new-libini_config-API.patch
Type: text/x-patch
Size: 33710 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130419/26359e0d/attachment.bin>


More information about the sssd-devel mailing list