On (26/05/14 15:53), Jakub Hrozek wrote:
>On Sat, May 24, 2014 at 02:13:28PM +0200, Lukas Slebodnik wrote:
>> On (23/05/14 17:39), Jakub Hrozek wrote:
>> >On Fri, May 23, 2014 at 11:56:32AM +0200, Jakub Hrozek wrote:
>> >> On Fri, May 23, 2014 at 08:31:49AM +0200, Lukas Slebodnik wrote:
>> >> > On (21/05/14 22:53), Jakub Hrozek wrote:
>> >> > >Hi,
>> >> > >
>> >> > >the attached patches implement the
>> >> > >org.freedesktop.DBus.Properties.GetAll method for retrieving primitive
>> >> > >types and their arrays.
>> >> > >
>> >> > >The patchset build on the previous set (currently still on review) that
>> >> > >implements the .Get method. The GetAll handler simply walks through the
>> >> > >existing getters and retrieves the results, storing them in a single
>> >> > >variant.
>> >> > >
>> >> > >The patches include a unit test that tests all currently supported
>> >> > >combinations of primitive types and array of primitive types.
>> >> >
>> >> > >From fc4b278c0914b2c7001393e471f4f6f0b532ba54 Mon Sep 17 00:00:00 2001
>> >> > >From: Jakub Hrozek <[email protected]>
>> >> > >Date: Wed, 21 May 2014 16:48:45 +0200
>> >> > >Subject: [PATCH 1/5] SBUS: Add utility function
>> >> > >sbus_add_variant_to_dict
>> >> > >
>> >> > >---
>> >> > > src/sbus/sssd_dbus.h | 5 ++
>> >> > > src/sbus/sssd_dbus_properties.c | 116
>> >> > > ++++++++++++++++++++++++++++++++++++++++
>> >> > > 2 files changed, 121 insertions(+)
>> >> > >
>> >> > src/sbus/sssd_dbus_properties.c:287:41: error: no member named
>> >> > 'invoker_get_all' in
>> >> > 'struct sbus_interface_meta'
>> >> > meta->invoker_get_all);
>> >> > ~~~~ ^
>> >> > It is introduced in 4th aptch
>> >> > "SBUS: Implement org.freedesktop.DBus.Properties.GetAll for primitive
>> >> > types"
>> >> >
>> >> > >From 32d84445012991dba38ee5b60f5614aca70b889d Mon Sep 17 00:00:00 2001
>> >> > >From: Jakub Hrozek <[email protected]>
>> >> > >Date: Wed, 21 May 2014 20:10:23 +0200
>> >> > >Subject: [PATCH 5/5] SBUS: Add org.freedesktop.DBus.Properties.GetAll
>> >> > >to
>> >> > > Introspection
>> >> >
>> >> > FAIL: sbus_tests
>> >> >
>> >> > sbys_tests.log file is attached.
>> >> >
>> >> > LS
>> >>
>> >> Ah, I didn't amend the test to include GetAll in introspection like I
>> >> did with Get, thanks. I'll prepare a new set.
>> >
>> >Hi,
>> >
>> >new patches are attached, but only the Introspection patch had changed
>> >-- I added the GetAll XML to the unit test.
>>
>> >From 9511be0f60affc2909ec2388f0d1ab5cc8487647 Mon Sep 17 00:00:00 2001
>> >From: Jakub Hrozek <[email protected]>
>> >Date: Wed, 21 May 2014 21:29:15 +0200
>> >Subject: [PATCH 4/5] SBUS: Implement org.freedesktop.DBus.Properties.GetAll
>> > for primitive types
>> >
>> >This patch implements the GetAll method of the
>> >org.freedesktop.DBus.Properties interface by iterating over the
>> >available getters and putting all the results into a single getter.
>> >
>> >The patch includes a unit test that exercies all currently supported
>> >array types.
>> >---
>> > src/monitor/monitor_iface_generated.c | 2 +
>> > src/providers/data_provider_iface_generated.c | 2 +
>> > src/responder/ifp/ifp_iface_generated.c | 1 +
>> > src/sbus/sbus_codegen | 96 +++++-
>> > src/sbus/sssd_dbus_meta.h | 1 +
>> > src/tests/sbus_codegen_tests.c | 299 +++++++++++++++++++
>> > src/tests/sbus_codegen_tests_generated.c | 415
>> > +++++++++++++++++++++++++-
>> > 7 files changed, 806 insertions(+), 10 deletions(-)
>> >
>>
>> //snip
>>
>> >diff --git a/src/sbus/sssd_dbus_meta.h b/src/sbus/sssd_dbus_meta.h
>> >index
>> >f25b1ac28abf1b1ef53fa54a090fa84ef868ab77..2cb9e95e3e12da55b1f101e076a0e05203984215
>> > 100644
>> >--- a/src/sbus/sssd_dbus_meta.h
>> >+++ b/src/sbus/sssd_dbus_meta.h
>> >@@ -80,6 +80,7 @@ struct sbus_interface_meta {
>> > const struct sbus_method_meta *methods;
>> > const struct sbus_signal_meta *signals;
>> > const struct sbus_property_meta *properties;
>> >+ sbus_method_invoker_fn invoker_get_all;
>> > };
>> >
>> Move this change to the 1st patch.
>
>Fixed.
>
>>
>> There is little problem with introspection.
>>
>> sh# gdbus introspect --system --dest org.freedesktop.sssd.infopipe
>> --object-path / --recurse
>>
>> ==11539== Jump to the invalid address stated on the next line
>> ==11539== at 0x0: ???
>> ==11539== by 0x7C3E25C: sbus_request_invoke_or_finish
>> (sssd_dbus_request.c:69)
>> ==11539== by 0x7C3DC47: sbus_properties_dispatch
>> (sssd_dbus_properties.c:361)
>> ==11539== by 0x7C3BD87: sbus_handler_got_caller_id
>> (sssd_dbus_connection.c:524)
>> ==11539== by 0x4C2D753: tevent_common_loop_immediate
>> (tevent_immediate.c:135)
>> ==11539== by 0x4C31FBD: epoll_event_loop_once (tevent_epoll.c:908)
>> ==11539== by 0x4C306B6: std_event_loop_once (tevent_standard.c:112)
>> ==11539== by 0x4C2CF2C: _tevent_loop_once (tevent.c:530)
>> ==11539== by 0x4C2D0CA: tevent_common_loop_wait (tevent.c:634)
>> ==11539== by 0x4C30656: std_event_loop_wait (tevent_standard.c:138)
>> ==11539== by 0x7C42C12: server_loop (server.c:587)
>> ==11539== by 0x10C571: main (ifpsrv.c:397)
>> ==11539== Address 0x0 is not stack'd, malloc'd or (recently) free'd
>> ==11539==
>> ==11539==
>> ==11539== Process terminating with default action of signal 11 (SIGSEGV)
>> ==11539== Bad permissions for mapped region at address 0x0
>> ==11539== at 0x0: ???
>> ==11539== by 0x7C3E25C: sbus_request_invoke_or_finish
>> (sssd_dbus_request.c:69)
>> ==11539== by 0x7C3DC47: sbus_properties_dispatch
>> (sssd_dbus_properties.c:361)
>> ==11539== by 0x7C3BD87: sbus_handler_got_caller_id
>> (sssd_dbus_connection.c:524)
>> ==11539== by 0x4C2D753: tevent_common_loop_immediate
>> (tevent_immediate.c:135)
>> ==11539== by 0x4C31FBD: epoll_event_loop_once (tevent_epoll.c:908)
>> ==11539== by 0x4C306B6: std_event_loop_once (tevent_standard.c:112)
>> ==11539== by 0x4C2CF2C: _tevent_loop_once (tevent.c:530)
>> ==11539== by 0x4C2D0CA: tevent_common_loop_wait (tevent.c:634)
>> ==11539== by 0x4C30656: std_event_loop_wait (tevent_standard.c:138)
>> ==11539== by 0x7C42C12: server_loop (server.c:587)
>> ==11539== by 0x10C571: main (ifpsrv.c:397)
>> ==11539==
>> ==11539== HEAP SUMMARY:
>> ==11539== in use at exit: 93,205 bytes in 449 blocks
>> ==11539== total heap usage: 3,296 allocs, 2,847 frees, 440,463 bytes
>> allocated
>>
>
>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.
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 */
};
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 :-)
LS
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
