[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