[SSSD] [INI] Patches for the INI configuration modification

Jakub Hrozek jhrozek at redhat.com
Tue Jun 23 09:25:49 UTC 2015


On Tue, Jun 23, 2015 at 10:36:30AM +0200, Lukas Slebodnik wrote:
> Patches 1-16 are Dimitri's patches
> and other patches are my bug fixes + unit test.
> It's quite big but it covers all functionality of function
> ini_config_add_str_value which is reused by other public functions.

I'll only review Lukas' fixes:
 * ini_config_ut: enable verbose mode with env variable - ACK

 * collection: Add new function col_remove_item_with_cb - the changes
   are fine, but do you want to also change the TRACE_FLOW_STRING from
   Exit/Exit to Enter/Exit in col_remove_item_with_cb()?

   Also col_remove_item() has no tracing, but I guess that's OK since
   it's just a wrapper.

 * INI: Fix memory leak with INI_VA_CLEAN - ACK, value_destroy() would
   now be called

 * COLLECTION: Return the last duplicate for big index - sorry, I'm too
   detached from the code to understand the change, can you add a comment
   what does this block do? Style-wise, why did you use Yoda-condition?

 * INI: Fix adding string with INI_VA_MODADD_E and big index - this
   seems a bit strange to me, wouldn't it be better to
   INI_VA_MODADD/INI_VA_MODADD_E cases? In particular, using exact case
   always to 1 even for INI_VA_MODADD seems a bit odd?

 * INI: Add check based test ini_configmod_ut_check
 Seems to cover the code well, runs fine and has no leaks:
 ==18028==   total heap usage: 20,176 allocs, 20,176 frees, 4,786,296
 bytes allocated



More information about the sssd-devel mailing list