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(a)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(a)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@entry=0xd525d8) at
src/util/sss_ini.c:101
#2 0x0000000000414b73 in confdb_init_db (config_file=config_file@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.