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.
On 05/21/2014 10:53 PM, 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.
Ack.
Thanks for squashing my patches in.
On Thu, May 22, 2014 at 07:39:06PM +0200, Pavel Březina wrote:
On 05/21/2014 10:53 PM, 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.
Ack.
Thanks for squashing my patches in.
Would you prefer to have any of them separately to preserve attribution?
I wasn't sure if the review and the patches themselves wouldn't look out of context, but I'm all for giving credit.
The only one I split was the large sbus helper for arrays.
On 05/22/2014 08:37 PM, Jakub Hrozek wrote:
On Thu, May 22, 2014 at 07:39:06PM +0200, Pavel Březina wrote:
On 05/21/2014 10:53 PM, 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.
Ack.
Thanks for squashing my patches in.
Would you prefer to have any of them separately to preserve attribution?
No, I didn't meant it in a grumbling way. The changes were little so it is better to have it in one patch - you just saved my time :-)
I wasn't sure if the review and the patches themselves wouldn't look out of context, but I'm all for giving credit.
The only one I split was the large sbus helper for arrays.
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@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@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
On (23/05/14 08:31), 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@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@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
Ii is probably unrelated to failed test, but there is another gcc warning introduced in 4th patch.
src/tests/sbus_codegen_tests.c: In function ‘check_arr_prop’: sssd/src/tests/sbus_codegen_tests.c:1225: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
LS
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@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@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.
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@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@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.
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@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@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@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.
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
LS
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@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@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@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.
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.
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@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@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@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.
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 jhrozek@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@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@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.
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
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.
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
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
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik -
Pavel Březina