[SSSD] [PATCH] First batch of infrastructure patches for review
Lukas Slebodnik
lslebodn at redhat.com
Wed Feb 19 15:49:48 UTC 2014
On (19/02/14 14:01), Stef Walter wrote:
>On 15.01.2014 09:00, Stef Walter wrote:
>> Here's the updated patchset with the C99 struct initialization change.
>
>Rebased on master.
>
>Fixed issue when builddir was not as subdir of srcdir. Simpler codegen
>output code, instead of using print use a custom func.
>
>Stef
>From a0f6ed946b69a80d520bc48ca90bff107c73dec7 Mon Sep 17 00:00:00 2001
>From: Stef Walter <stefw at redhat.com>
>Date: Thu, 9 Jan 2014 23:04:45 +0100
>Subject: [PATCH 1/6] sbus: Add meta data structures and code generator
>
>These metadata structures hold the information about all the
>details of a DBus interface. They are typically generated from
>the canonical XML form of the DBus interface, although they
>may also be hand crafted.
>
>Add some handy functions for looking up methods, props, signals,
>in the metadata of an interface. Currently lookups are just done
>by looking through an array. If performance becomes an issue (ie:
>very large interfaces) it would be really easy to sort things
>and use bsearch().
>
>Later commits will include some definitions using this metadata
>and related functions.
>
>DBus interfaces are defined here:
>
>http://dbus.freedesktop.org/doc/dbus-specification.html#introspection-format
>
>The introspection data format has become the standard way to represent a
>DBus interface. For many examples see /usr/share/dbus-1/interfaces/ on a
>typical linux machine.
>
>A word about annotations. These are extra flags or values that can be
>assigned to anything. So far, the codegen supports this annotation:
>
>org.freedesktop.DBus.GLib.CSymbol
> - An annotation specified in the specification that tells us what C symbol
> to generate for a given interface or method. By default the codegen will
> build up a symbol name from the DBus name.
>
>It is possible to confuse the code generator into producing invalid
>C code (with strange method names, for example), but the C compiler
>catches such silliness right away.
>
>Add tests testing basic features of the codegen and poking through
>the metadata it creates. Also test the metadata lookup functions.
>
>Generated code is checked in for easy discovery.
>
>An example of the XML interface definitions can be found at:
>
> src/tests/sbus_codegen_tests.xml
>
>And an example of the generated header can be found here:
>
> src/tests/sbus_codegen_tests_generated.h
>---
> Makefile.am | 47 ++-
> src/sbus/sbus_codegen | 506 +++++++++++++++++++++++++++++++
> src/sbus/sssd_dbus_meta.c | 64 ++++
> src/sbus/sssd_dbus_meta.h | 82 +++++
> src/tests/sbus_codegen_tests.c | 166 ++++++++++
> src/tests/sbus_codegen_tests.xml | 45 +++
> src/tests/sbus_codegen_tests_generated.c | 79 +++++
> src/tests/sbus_codegen_tests_generated.h | 24 ++
> 8 files changed, 1012 insertions(+), 1 deletion(-)
> create mode 100755 src/sbus/sbus_codegen
> create mode 100644 src/sbus/sssd_dbus_meta.c
> create mode 100644 src/sbus/sssd_dbus_meta.h
> create mode 100644 src/tests/sbus_codegen_tests.c
> create mode 100755 src/tests/sbus_codegen_tests.xml
> create mode 100644 src/tests/sbus_codegen_tests_generated.c
> create mode 100644 src/tests/sbus_codegen_tests_generated.h
>
<snip>
>diff --git a/src/sbus/sbus_codegen b/src/sbus/sbus_codegen
>new file mode 100755
>index 0000000..95a10a7
>--- /dev/null
>+++ b/src/sbus/sbus_codegen
>@@ -0,0 +1,506 @@
>+#!/bin/python
^^^^^^^^^^^
Please use /usr/bin/python, because we try to build nightly master on RHEL6.
>+
>+#
>+# Authors:
>+# Stef Walter <stefw at redhat.com>
>+#
>+# Copyright (C) 2014 Red Hat
>+#
<snip>
>From 032f6ae1239e03e19ebd89e57053a3f95b050579 Mon Sep 17 00:00:00 2001
>From: Stef Walter <stefw at redhat.com>
>Date: Fri, 10 Jan 2014 11:50:43 +0100
>Subject: [PATCH 3/6] nss: Stop using one DBus interface with totally different
> methods
>
>This is an incorrect use of DBus, where we use a single interface
>name with completely different sets of methods.
>
>Easily fixed.
>
>Once the vtable stuff is in use then this would be automatically
>detected and fail to build.
>---
> src/providers/data_provider.h | 1 +
> src/providers/data_provider_be.c | 2 +-
> src/responder/nss/nsssrv.c | 2 +-
> 3 files changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/src/providers/data_provider.h b/src/providers/data_provider.h
>index f88637c..c33445c 100644
>--- a/src/providers/data_provider.h
>+++ b/src/providers/data_provider.h
>@@ -60,6 +60,7 @@
> /* this is a reverse method sent from providers to
> * the nss responder to tell it to update the mmap
> * cache */
>+#define DP_REV_INTERFACE "org.freedesktop.sssd.dataprovider_rev"
> #define DP_REV_METHOD_UPDATE_CACHE "updateCache"
> #define DP_REV_METHOD_INITGR_CHECK "initgrCheck"
>
>diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
>index bfb776c..2e64226 100644
>--- a/src/providers/data_provider_be.c
>+++ b/src/providers/data_provider_be.c
>@@ -857,7 +857,7 @@ static void acctinfo_initgroups_callback(struct be_req *be_req,
> /* Set up null request */
> msg = dbus_message_new_method_call(NULL,
> DP_PATH,
>- DP_INTERFACE,
>+ DP_REV_INTERFACE,
> DP_REV_METHOD_INITGR_CHECK);
> if (!msg) {
> DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory?!\n");
>diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c
>index 9d0c996..d258dfc 100644
>--- a/src/responder/nss/nsssrv.c
>+++ b/src/responder/nss/nsssrv.c
>@@ -379,7 +379,7 @@ static struct sbus_method nss_dp_methods[] = {
> };
>
> struct sbus_interface nss_dp_interface = {
>- DP_INTERFACE,
>+ DP_REV_INTERFACE,
> DP_PATH,
> SBUS_DEFAULT_VTABLE,
> nss_dp_methods,
>--
>1.8.5.3
>
>From d73391569a608f8ad51ff86e4abe2fdcb31a9560 Mon Sep 17 00:00:00 2001
>From: Stef Walter <stefw at redhat.com>
>Date: Fri, 10 Jan 2014 08:58:12 +0100
>Subject: [PATCH 4/6] sbus: Rework sbus to use interface metadata and vtables
>
>Previous commits added support for interface metadata and
>handler vtables. This commit ports sbus_dbus_connection to
>use them.
>
>Port the internal uses of dbus to use the new scheme in a
>very minimal way. Further cleanup is possible here.
>
>This commit provides basic definitions of the internal
>dbus interfaces. The interfaces aren't fully defined, as the
>handlers will continue to unpack manually, and often overload
>DBus methods with different arguments (which is rather
>unorthodox, but not the end of the world).
>---
<snip>
>diff --git a/src/sbus/sssd_dbus_connection.c b/src/sbus/sssd_dbus_connection.c
>index eb07b8d..d319674 100644
>--- a/src/sbus/sssd_dbus_connection.c
>+++ b/src/sbus/sssd_dbus_connection.c
<snip>
>@@ -427,26 +437,32 @@ DBusHandlerResult sbus_message_handler(DBusConnection *dbus_conn,
> return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
>
> /* Validate the method interface */
>- if (strcmp(msg_interface, intf_p->intf->interface) == 0) {
>- found = 0;
>- for (i = 0; intf_p->intf->methods[i].method != NULL; i++) {
>- if (strcmp(method, intf_p->intf->methods[i].method) == 0) {
>- found = 1;
>- ret = intf_p->intf->methods[i].fn(message, intf_p->conn);
>- if (ret != EOK) {
>- return sbus_reply_internal_error(message, intf_p->conn);
>- }
>- break;
>- }
>- }
>-
>- if (!found) {
>+ interface = intf_p->intf->vtable->meta;
>+ if (strcmp(msg_interface, interface->name) == 0) {
>+ handler_fn = NULL;
>+ method = sbus_meta_find_method(interface, msg_method);
>+ if (method && method->vtable_offset)
>+ handler_fn = VTABLE_FUNC(intf_p->intf->vtable, method->vtable_offset);
>+
>+ if (!method) {
> /* Reply DBUS_ERROR_UNKNOWN_METHOD */
> DEBUG(SSSDBG_CRIT_FAILURE,
>- "No matching method found for %s.\n", method);
>+ "No matching method found for %s.\n", msg_method);
> reply = dbus_message_new_error(message, DBUS_ERROR_UNKNOWN_METHOD, NULL);
> sbus_conn_send_reply(intf_p->conn, reply);
> dbus_message_unref(reply);
>+
>+ } else if (!handler_fn) {
>+ /* Reply DBUS_ERROR_NOT_SUPPORTED */
>+ DEBUG(1, ("No matching handler found for %s.\n", msg_method));
You forgot to update this debug message.
src/sbus/sssd_dbus_connection.c:457:23: error: expression result unused [-Werror,-Wunused-value]
DEBUG(1, ("No matching handler found for %s.\n", msg_method));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/util/util.h:127:18: note: expanded from macro 'DEBUG'
format, ##__VA_ARGS__); \
^
src/sbus/sssd_dbus_connection.c:457:22: error: format string is not a string literal
(potentially insecure) [-Werror,-Wformat-security]
DEBUG(1, ("No matching handler found for %s.\n", msg_method));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/util/util.h:127:18: note: expanded from macro 'DEBUG'
format, ##__VA_ARGS__); \
^
LS
More information about the sssd-devel
mailing list