[SSSD] [PATCH] sss_config: alter configuration file

Pavel Březina pbrezina at redhat.com
Wed Apr 23 09:03:25 UTC 2014


On 04/22/2014 11:21 AM, Lukas Slebodnik wrote:
> On (22/04/14 10:29), Pavel Březina wrote:
>> Hi,
>> I'm sending some patches that I'll use for OpenLMI provider. It
>> supports few modifications of sssd.conf through augeas.
>>
>> For the moment, I think we should not bound to any particular API so
>> even though I made it a separate object, I don't have any intentions
>> to make it publicly usable library.
>>
>> This code will be used from D-Bus responder. I may extend the API if needed.
>>
>> Unit tests are attached.
>
>> +    option_path = build_option_path(tmp_ctx, ctx, section, option);
>> +    if (option_path == NULL) {
>            ret = EINVAL???
>
> src/util/sss_config.c:203:9: error: variable 'ret' is used uninitialized whenever 'if' condition
>        is true [-Werror,-Wsometimes-uninitialized]
>      if (option_path == NULL) {
>          ^~~~~~~~~~~~~~~~~~~
> src/util/sss_config.c:233:12: note: uninitialized use occurs here
>      return ret;
>             ^~~

Thanks, fixed.

>> @@ -435,6 +435,7 @@ dist_noinst_HEADERS = \
>>      src/util/sss_ssh.h \
>>      src/util/sss_ini.h \
>>      src/util/sss_format.h \
>> +    src/util/sss_config.h \
>>      src/util/refcount.h \
>>      src/util/find_uid.h \
>>      src/util/user_info_msg.h \
>> @@ -577,6 +578,17 @@ pkglib_LTLIBRARIES += libsss_child.la
>> libsss_child_la_SOURCES = src/util/child_common.c
>> libsss_child_la_LDFLAGS = -avoid-version
>>
> ^^^^
> trailing spaces.
>
> sh$ git am ~/000*
> Applying: sss_config: the code
> Applying: sss_config: build
> /dev/shm/sssd/.git/rebase-apply/patch:65: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.
> Applying: sss_config: unit tests
> /dev/shm/sssd/.git/rebase-apply/patch:25: trailing whitespace.
>
> warning: 1 line adds whitespace errors.

Fixed.

>
>> +pkglib_LTLIBRARIES += libsss_config.la
>    ^^^^^^
>    libsss_config.so will be installed in private directory /usr/lib64/sssd
>    If augeas opens this library with dlopen it should be installed in different
>    directory.

It is not dlopened. We use augeas libs, not the other way around.

>> +libsss_config_la_SOURCES = \
>> +    src/util/sss_config.c
>> +libsss_config_la_CFLAGS = \
>> +	$(AM_CFLAGS) \
>    ^^^^
>    '\t' instead of spaces.

Fixed.

>> +    $(AUGEAS_CFLAGS)
>> +libsss_config_la_LIBADD = \
>> +    $(AUGEAS_LIBS)
>
> ./.libs/libsss_config.so: undefined reference to `string_in_list'
> ./.libs/libsss_config.so: undefined reference to `split_on_separator'
> ./.libs/libsss_config.so: undefined reference to `add_string_to_list'
>
> add $(SSSD_INTERNAL_LTLIBS) to LIBADD

Fixed.

>> +++ b/src/external/libaugeas.m4
>> @@ -0,0 +1,10 @@
>> +AC_SUBST(AUGEAS_OBJ)
>> +AC_SUBST(AUGEAS_CFLAGS)
>> +AC_SUBST(AUGEAS_LIBS)
>> +
>> +PKG_CHECK_MODULES(AUGEAS,
>> +    augeas >= 1.0.0,
>> +    ,
>> +    AC_MSG_ERROR("Please install augeas-devel")
>> +    )
>> +
>> --
>> 1.7.11.7
>>
>
> Could be this optional dependency like a cifs-idmap-plugin?
>      src/external/cifsidmap.m4

This library is intended to be used from dbus responder only, for the 
time being.

Do we want to build it by default and add an option to disable it?
Do we want to build it only when dbus responder is enabled?
Do we want to allow the possibility to build dbus responder without 
support of configuration changes?

 > You added new build time dependency, but spec file was not change.
> You created new dynamic library(module) but it was not added into dlopen tests.

Will do, once we decide the build condition.

Thanks for the review.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-sss_config-the-code.patch
Type: text/x-patch
Size: 15220 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140423/f761f0aa/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-sss_config-build.patch
Type: text/x-patch
Size: 2225 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140423/f761f0aa/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-sss_config-unit-tests.patch
Type: text/x-patch
Size: 22830 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140423/f761f0aa/attachment-0002.bin>


More information about the sssd-devel mailing list