2012-04-18 16:47:58

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ 1/5] gdbus: return if method signature is malformed

---
gdbus/object.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/gdbus/object.c b/gdbus/object.c
index 8bc12f5..7a94156 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -107,6 +107,10 @@ static void print_arguments(GString *gstr, const char *sig,
break;
}

+ if (!complete) {
+ error("Unexpected signature: %s", sig);
+ return;
+ }

if (direction)
g_string_append_printf(gstr,
--
1.7.10



2012-04-18 18:57:08

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH BlueZ 3/5] gdbus: add decorated DBus signature for methods

On Wed, Apr 18, 2012 at 1:48 PM, Lucas De Marchi
<[email protected]> wrote:
> To generate better introspection data we need to keep the name of each
> argument in GDBusMethodTable. With decorated signatures we can save
> them, while being compatible with the previous format. It is supposed to
> be used as following:
>
> s[argname1]o[argname2]
>
> We can't simply change signature to this format because we need to be
> able to rapidly check if messages have that same signature when they
> arrive: that is a much hotter path than the Introspect call, which
> this patch tries to solve.
>
> Therefore, when registering the interface we create the undecorated
> signature and save on the same structure, that is used by everybody.
> Introspect can later make use of the decorated version, exporting
> argument names.
> ---
> ?gdbus/gdbus.h ?| ? ?7 +++++--
> ?gdbus/object.c | ? 34 +++++++++++++++++++++++++++++++++-
> ?2 files changed, 38 insertions(+), 3 deletions(-)

Don't bother too much with the current implementation, only with the
idea of adding the arguments using "decorated signatures". This
implementation has some bugs that I need to work out yet. Same apply
for patches 4 and 5.

Lucas De Marchi

2012-04-18 16:48:02

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ 5/5] manager: set argument name for introspection

Now we have the following extra 'name=pattern' as response to
org.freedesktop.DBus.Introspectable.Introspect:

...
<method name="FindAdapter">
<arg name="pattern" type="s" direction="in"/>
<arg type="o" direction="out"/>
</method>
...
---
src/manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/manager.c b/src/manager.c
index 6244516..5ee1054 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -199,7 +199,7 @@ static DBusMessage *get_properties(DBusConnection *conn,
static GDBusMethodTable manager_methods[] = {
{ "GetProperties", "", "a{sv}",get_properties },
{ "DefaultAdapter", "", "o", default_adapter },
- { "FindAdapter", "s", "o", find_adapter },
+ { "FindAdapter", "s[pattern]", "o", find_adapter },
{ "ListAdapters", "", "ao", list_adapters,
G_DBUS_METHOD_FLAG_DEPRECATED},
{ }
--
1.7.10


2012-04-18 16:48:01

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ 4/5] gdbus: use argument name in method introspection

---
gdbus/object.c | 34 +++++++++++++++++++++++++++++-----
1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index bfd1873..3f82865 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -66,6 +66,7 @@ static void print_arguments(GString *gstr, const char *sig,
int i;

for (i = 0; sig[i]; i++) {
+ char name[64];
char type[32];
int struct_level, dict_level;
unsigned int len;
@@ -113,15 +114,38 @@ static void print_arguments(GString *gstr, const char *sig,
}

type[len + 1] = '\0';
+ len = 0;
+
+ /* Check if there is an arg name */
+ if (sig[i + 1] == '[') {
+ len = sizeof(" name=\"") - 1;
+ memcpy(name, " name=\"", len);
+
+ for (i += 2; len < (sizeof(name) - 1); i++, len++) {
+ if (sig[i] == '\0') {
+ error("Unexpected signature: %s", sig);
+ return;
+ }
+
+ if (sig[i] == ']')
+ break;
+
+ name[len] = sig[i];
+ }
+
+ name[len++] = '\"';
+ }
+
+ name[len] = '\0';

if (direction)
g_string_append_printf(gstr,
- "\t\t\t<arg type=\"%s\" direction=\"%s\"/>\n",
- type, direction);
+ "\t\t\t<arg%s type=\"%s\" direction=\"%s\"/>\n",
+ name, type, direction);
else
g_string_append_printf(gstr,
- "\t\t\t<arg type=\"%s\"/>\n",
- type);
+ "\t\t\t<arg%s type=\"%s\"/>\n",
+ name, type);
}
}

@@ -137,7 +161,7 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
else {
g_string_append_printf(gstr, "\t\t<method name=\"%s\">\n",
method->name);
- print_arguments(gstr, method->signature, "in");
+ print_arguments(gstr, method->decorated_signature, "in");
print_arguments(gstr, method->reply, "out");
g_string_append_printf(gstr, "\t\t</method>\n");
}
--
1.7.10


2012-04-18 16:47:59

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ 2/5] gdbus: do not call memset for terminating NUL

---
gdbus/object.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 7a94156..55b47f2 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -72,7 +72,6 @@ static void print_arguments(GString *gstr, const char *sig,

complete = FALSE;
struct_level = dict_level = 0;
- memset(type, 0, sizeof(type));

/* Gather enough data to have a single complete type */
for (len = 0; len < (sizeof(type) - 1) && sig[i]; len++, i++) {
@@ -112,6 +111,8 @@ static void print_arguments(GString *gstr, const char *sig,
return;
}

+ type[len + 1] = '\0';
+
if (direction)
g_string_append_printf(gstr,
"\t\t\t<arg type=\"%s\" direction=\"%s\"/>\n",
--
1.7.10


2012-04-18 16:48:00

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ 3/5] gdbus: add decorated DBus signature for methods

To generate better introspection data we need to keep the name of each
argument in GDBusMethodTable. With decorated signatures we can save
them, while being compatible with the previous format. It is supposed to
be used as following:

s[argname1]o[argname2]

We can't simply change signature to this format because we need to be
able to rapidly check if messages have that same signature when they
arrive: that is a much hotter path than the Introspect call, which
this patch tries to solve.

Therefore, when registering the interface we create the undecorated
signature and save on the same structure, that is used by everybody.
Introspect can later make use of the decorated version, exporting
argument names.
---
gdbus/gdbus.h | 7 +++++--
gdbus/object.c | 34 +++++++++++++++++++++++++++++++++-
2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index a0583e6..0b8a556 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -82,13 +82,16 @@ typedef enum {
G_DBUS_SECURITY_FLAG_ALLOW_INTERACTION = (1 << 2),
} GDBusSecurityFlags;

+#define MAX_SIGNATURE_SIZE 32
+
typedef struct {
const char *name;
- const char *signature;
+ const char *decorated_signature;
const char *reply;
GDBusMethodFunction function;
GDBusMethodFlags flags;
unsigned int privilege;
+ char signature[MAX_SIGNATURE_SIZE];
} GDBusMethodTable;

typedef struct {
@@ -112,7 +115,7 @@ typedef struct {

gboolean g_dbus_register_interface(DBusConnection *connection,
const char *path, const char *name,
- const GDBusMethodTable *methods,
+ GDBusMethodTable *methods,
const GDBusSignalTable *signals,
const GDBusPropertyTable *properties,
void *user_data,
diff --git a/gdbus/object.c b/gdbus/object.c
index 55b47f2..bfd1873 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -25,6 +25,7 @@
#include <config.h>
#endif

+#include <assert.h>
#include <stdio.h>
#include <string.h>

@@ -501,6 +502,36 @@ static GDBusMethodTable introspect_methods[] = {
{ }
};

+static void undecorate_methods(GDBusMethodTable *m)
+{
+ int pushed;
+
+ for (pushed = 0; m->name; m++) {
+ const char *c;
+
+ for (c = m->decorated_signature; pushed < MAX_SIGNATURE_SIZE
+ && *c != '\0'; c++) {
+ switch (*c) {
+ case '[':
+ while (*c != ']' && *c != '\0')
+ c++;
+
+ /* end of string without matching ']' */
+ assert(*c == ']');
+ break;
+ case ']':
+ /* ']' without '[' */
+ assert(0);
+ break;
+ default:
+ m->signature[pushed++] = *c;
+ }
+ }
+
+ assert(pushed != MAX_SIGNATURE_SIZE);
+ }
+}
+
static void add_interface(struct generic_data *data, const char *name,
const GDBusMethodTable *methods,
const GDBusSignalTable *signals,
@@ -676,7 +707,7 @@ fail:

gboolean g_dbus_register_interface(DBusConnection *connection,
const char *path, const char *name,
- const GDBusMethodTable *methods,
+ GDBusMethodTable *methods,
const GDBusSignalTable *signals,
const GDBusPropertyTable *properties,
void *user_data,
@@ -693,6 +724,7 @@ gboolean g_dbus_register_interface(DBusConnection *connection,
return FALSE;
}

+ undecorate_methods(methods);
add_interface(data, name, methods, signals,
properties, user_data, destroy);

--
1.7.10