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

Jakub Hrozek jhrozek at redhat.com
Mon May 26 13:59:12 UTC 2014


On Mon, May 26, 2014 at 03:53:26PM +0200, 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 <jhrozek at redhat.com>
> > >> > >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 <jhrozek at redhat.com>
> > >> > >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 <jhrozek at redhat.com>
> > >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.

btw returning an empty array instead of, for example, returning an
error, was suggested by Marius Vollmer.



More information about the sssd-devel mailing list