[SSSD] [PATCH] Implement org.freedesktop.DBus.Properties.GetAll for primitive types

Jakub Hrozek jhrozek at redhat.com
Tue May 27 08:47:19 UTC 2014


On Mon, May 26, 2014 at 09:16:11PM +0200, Lukas Slebodnik wrote:
> On (26/05/14 18:39), Jakub Hrozek wrote:
> >On Mon, May 26, 2014 at 05:30:05PM +0200, Lukas Slebodnik wrote:
> >> >Thanks, I didn't see this earlier because I never used --recurse.
> >> >
> >> >I modified the codegen a bit so than an invoker is called even if there
> >> >are no properties. The invoker creates an empty array, adds nothing (the
> >> >property getters would be inserted here by codegen) and returns.
> >> >
> >> >The other way I was considering was checking for properties in the
> >> >getall_handler in sssd, but I prefer to let the generated code do as
> >> >much work as possible.
> >> 
> >> Crash is fixed.
> >> Static analysers did not found any issues.
> >> 
> >> There are few gcc warnings.
> >
> >Ah, I haven't seen any with clang..
> >
> >> 
> >>   CC       src/tests/sbus_tests-sbus_tests.o
> >> src/tests/sbus_tests.c:105:1: error: missing initializer for field ‘invoker_get_all’ of ‘const struct sbus_interface_meta’ [-Werror=missing-field-initializers]
> >>  };
> >>  ^
> >> In file included from ../sssd/src/tests/sbus_tests.c:34:0:
> >> src/sbus/sssd_dbus_meta.h:83:28: note: ‘invoker_get_all’ declared here
> >>      sbus_method_invoker_fn invoker_get_all;
> >>                             ^
> >> cc1: all warnings being treated as errors
> >> 
> >> Warning can be fixed with diff:
> >> diff --git a/src/tests/sbus_tests.c b/src/tests/sbus_tests.c
> >> index 31b91c4..fe6ee05 100644
> >> --- a/src/tests/sbus_tests.c
> >> +++ b/src/tests/sbus_tests.c
> >> @@ -102,6 +102,7 @@ const struct sbus_interface_meta pilot_meta = {
> >>      pilot_methods,
> >>      NULL, /* no signals */
> >>      NULL, /* no properties */
> >> +    NULL, /* invoker function */
> >>  };
> >
> >Thank you, I merged the diff.
> >
> >>  
> >>  static int blink_handler(struct sbus_request *req, void *data)
> >> 
> >> 
> >> And the second warning:
> >> src/tests/sbus_codegen_tests.c: In function ‘check_arr_prop’:
> >> src/tests/sbus_codegen_tests.c:1226:30: error: ‘strings’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >>              ck_assert_str_eq(strings[1], pilot_path_array[1]);
> >>                               ^
> >> cc1: all warnings being treated as errors
> >> 
> >> I am not sure about solution :-)
> >
> >I think the warning was bogus. I simply initialized strings to NULL and
> >then added some asserts to make sure that strings can be dereferenced
> >when we get to the comparison. I hope this would also silence gcc, at
> >least I haven't seen any warning with my test setup.
> 
> Thank you for fixing all issues.
> 
> Pavel has already tested patches.
> ACK++
> 
> LS

Thanks for the review, pushed all to master:
    bdbb5cd11752d79488e5d87b908103322e010df1
    4f7f714e118e95896fac5239c7a8b529c39a4758
    886d29fced0bcc1668a3cb99a5bca66ea486b3a4
    2965f42fed62b77a5b0e7f6cd01c4be1072484f5
    c43ed56bc5d29e189e529235a3ba4bc17cd1a95f



More information about the sssd-devel mailing list