[Freeipa-devel] [PATCH 0077] Refactor settings subsystem

[Petr Spacek]

On 8.10.2012 15:29, Petr Spacek wrote:
> Hello,
>
> this patch refactors setting subsystem. After some experimenting I chosen
> simple implementation with static arrays, no RBT trees involved. Speed can be
> improved by reordering items in arrays.
>
> Commit message:
>
>     Refactor settings subsystem.
>
>      Settings are stored in tree of settings_set_t structures.
>      All settings should be accessed only through setting* functions.
>      Mutual exclusion during write is done by switching to single
>      thread mode.
>
>      Setting_get() function doesn't copy strings, so changing the
>      original string can lead to obscure bugs.
>      This way is okay as long as strings are not changed dynamically
>      at run-time.
>
>      Unknown setting in configuration file leads to failure rather than
>      silent ignoring it.
>
>      https://fedorahosted.org/bind-dyndb-ldap/ticket/53
>      https://fedorahosted.org/bind-dyndb-ldap/ticket/81

Re-based & amended & tested patch is attached. The main idea is the same.

> Adam, I'm still looking for way how to handle strings in settings. We have to
> prevent string change/deallocation as long as somebody has a pointer to the
> string (I mean pointer obtained through setting_get("name_of_setting") call).
>
> The only way which I can see is returning setting_string structure like
>
> setting_string struct {
>      isc_refcount_t counter;
>      isc_mem_t *mctx;
>      char *str;
> };

After some poking to isc_task_beginexclusive() I consider the "struct 
setting_string" idea redundant.

> Caller has to call setting_string_free() when string can be freed.
> Setting_string_free() will decrement counter by one and free whole structure
> if and only if counter reaches 0.
>
> Is it meaningful? Should I separate setting_get_* for each datatype? Or just
> for setting_get_string() and let setting_get() universal for integers and
> booleans?

I separated setting_get_bool/uint/str from the universal setting_get() to 
enable type checks.

-- 
Petr^2 Spacek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bind-dyndb-ldap-pspacek-0077-2-Refactor-settings-subsystem.patch
Type: text/x-patch
Size: 92978 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130219/f73d4774/attachment.bin>