[PATCH v0 0/2] gdbus introspection fixes

[PATCH v0 0/2] gdbus introspection fixes

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

Hi Johan,

During some connman debug session I found some small hickups in the
introspection code in gdbus. Marcel ask me to check with you first.

cheers,
daniel

Daniel Wagner (2):
  gdbus: invaldate_parent_data: walk the whole path down
  gdbus: Remove root node 'name' attribute in introspection

 gdbus/object.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

-- 
1.7.3.4

[PATCH v0 1/2] gdbus: invaldate_parent_data: walk the whole path down

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

Assume there is only one object registerd at "/". If we add a new
object at "/foo/bar" the introspection of "/" has to be updated. A new
node has to be added at "/".

invalidate_parent_data stops invaldating the whole path because the
boolean return value of dbus_connection_get_object_path_data is used
wrong.

If we get a TRUE just go on down in the path, if FALSE is return
dbus_connection_get_object_path_data has run out of memory.
---
 gdbus/object.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index afa904e..49006ec 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -475,12 +475,13 @@ static void invalidate_parent_data(DBusConnection *conn, const char *child_path)
 	if (!strlen(parent_path))
 		goto done;
 
-	if (!dbus_connection_get_object_path_data(conn, parent_path,
-							(void *) &data)) {
-		invalidate_parent_data(conn, parent_path);
+	if (dbus_connection_get_object_path_data(conn, parent_path,
+							(void *) &data) == FALSE) {
 		goto done;
 	}
 
+	invalidate_parent_data(conn, parent_path);
+
 	if (data == NULL)
 		goto done;
 
-- 
1.7.3.4

[PATCH v0 2/2] gdbus: Remove root node 'name' attribute in introspection

[Daniel Wagner]

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

generate_introspection_xml generates the root <node> tags with a
'name' attribute. This seems to be a valid attribute but it is not
consistent with the way the D-Bus daemon generates empty nodes.

For example if we register "/foo/bar", D-Bus daemon will generate for
"/foo" a introspection which looks like this:


<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN"
"http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
<node>
  <node name="bar"/>
</node>


and generate_introspection_xml generates for "/foo/bar":


<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN"
"http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
<node name="/foo/bar">
</node>


Just don't add the 'name' attribute to the root node.  The GLib
binding for D-Bus does it the same way.
---
 gdbus/object.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 49006ec..eaa2e1a 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -162,7 +162,7 @@ static void generate_introspection_xml(DBusConnection *conn,
 
 	gstr = g_string_new(DBUS_INTROSPECT_1_0_XML_DOCTYPE_DECL_NODE);
 
-	g_string_append_printf(gstr, "<node name=\"%s\">\n", path);
+	g_string_append_printf(gstr, "<node>\n");
 
 	for (list = data->interfaces; list; list = list->next) {
 		struct interface_data *iface = list->data;
-- 
1.7.3.4

Re: [PATCH v0 0/2] gdbus introspection fixes

[Jose Antonio Santos Cadenas]

Hi,

2011/1/14 Daniel Wagner <wagi@monom.org>:
> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
>
> Hi Johan,
>
> During some connman debug session I found some small hickups in the
> introspection code in gdbus. Marcel ask me to check with you first.
>

I've been thinking about the way that dbus is used in BlueZ and I am
concerned about why we don't use glib-dbus instead of dbus directly.
This way the dbus specific code will be maintained by glib-dbus.

I've thought it several times and I've never said anything because is
a big change and as everything was working I preferred not to make
noise with this. But now that some bugs appeared we can probably think
on this changing the BlueZ DBus code and all the plugins interfaces.
The only problem I see is that this will probably require a version
change because the way plugins expose their interface will change.

> cheers,
> daniel

Regards.

Jose.

Re: [PATCH v0 0/2] gdbus introspection fixes

[Elvis Pfützenreuter]

On Jan 15, 2011, at 11:05 AM, Jose Antonio Santos Cadenas wrote:

> Hi,
> 
> 2011/1/14 Daniel Wagner <wagi@monom.org>:
>> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
>> 
>> Hi Johan,
>> 
>> During some connman debug session I found some small hickups in the
>> introspection code in gdbus. Marcel ask me to check with you first.
>> 
> 
> I've been thinking about the way that dbus is used in BlueZ and I am
> concerned about why we don't use glib-dbus instead of dbus directly.
> This way the dbus specific code will be maintained by glib-dbus.
> 
> I've thought it several times and I've never said anything because is
> a big change and as everything was working I preferred not to make
> noise with this. But now that some bugs appeared we can probably think
> on this changing the BlueZ DBus code and all the plugins interfaces.
> The only problem I see is that this will probably require a version
> change because the way plugins expose their interface will change.


One (big) thing that makes me not to like glib-dbus is having to cope with GObject. ALso, it makes certain things more complicated and convoluted than they are in low-level API (e.g passing an array), it is poorly documented, and, incredibly enough, lacks things like the uint16 type.

My feeling goes the other way round: I wish we had the gdbus API *outside* BlueZ, as a library, so any C application could enjoy it.

Re: [PATCH v0 0/2] gdbus introspection fixes

[Jose Antonio Santos Cadenas]

Hi Elvis,

2011/1/15 Elvis Pfützenreuter <epx@signove.com>:
>
> On Jan 15, 2011, at 11:05 AM, Jose Antonio Santos Cadenas wrote:
>
>> Hi,
>>
>> 2011/1/14 Daniel Wagner <wagi@monom.org>:
>>> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
>>>
>>> Hi Johan,
>>>
>>> During some connman debug session I found some small hickups in the
>>> introspection code in gdbus. Marcel ask me to check with you first.
>>>
>>
>> I've been thinking about the way that dbus is used in BlueZ and I am
>> concerned about why we don't use glib-dbus instead of dbus directly.
>> This way the dbus specific code will be maintained by glib-dbus.
>>
>> I've thought it several times and I've never said anything because is
>> a big change and as everything was working I preferred not to make
>> noise with this. But now that some bugs appeared we can probably think
>> on this changing the BlueZ DBus code and all the plugins interfaces.
>> The only problem I see is that this will probably require a version
>> change because the way plugins expose their interface will change.
>
>
> One (big) thing that makes me not to like glib-dbus is having to cope with GObject. ALso, it makes certain things more complicated and convoluted than they are in low-level API (e.g passing an array), it is poorly documented, and, incredibly enough, lacks things like the uint16 type.

I've been studying the problem of types, specially uint16. You are
right, they are not implemented, and they never will because GObject
will never support them, nevertheless it is not very important since
other types like unit32 can hold them (read this email thread for more
info http://mail.gnome.org/archives/gtk-devel-list/2001-August/msg00492.html).
The array issue that you mentioned I think is not as complicated, it
uses specific Glib types, you only have to learn how to use them (and
the Glib documentation is quite good).

>
> My feeling goes the other way round: I wish we had the gdbus API *outside* BlueZ, as a library, so any C application could enjoy it.

It's a different view of the problem :), let's see if they are more
opinions out there.

Regards

Re: [PATCH v0 0/2] gdbus introspection fixes

[Luiz Augusto von Dentz]

Hi,

On Sat, Jan 15, 2011 at 3:31 PM, Elvis Pfützenreuter <epx@signove.com> wrote:
>
> On Jan 15, 2011, at 11:05 AM, Jose Antonio Santos Cadenas wrote:
>
>> Hi,
>>
>> 2011/1/14 Daniel Wagner <wagi@monom.org>:
>>> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
>>>
>>> Hi Johan,
>>>
>>> During some connman debug session I found some small hickups in the
>>> introspection code in gdbus. Marcel ask me to check with you first.
>>>
>>
>> I've been thinking about the way that dbus is used in BlueZ and I am
>> concerned about why we don't use glib-dbus instead of dbus directly.
>> This way the dbus specific code will be maintained by glib-dbus.
>>
>> I've thought it several times and I've never said anything because is
>> a big change and as everything was working I preferred not to make
>> noise with this. But now that some bugs appeared we can probably think
>> on this changing the BlueZ DBus code and all the plugins interfaces.
>> The only problem I see is that this will probably require a version
>> change because the way plugins expose their interface will change.
>
>
> One (big) thing that makes me not to like glib-dbus is having to cope with GObject. ALso, it makes certain things more complicated and convoluted than they are in low-level API (e.g passing an array), it is poorly documented, and, incredibly enough, lacks things like the uint16 type.

Exactly, GObject code doesn't really helps us, in fact I consider
glib-dbus very bad for using in a daemon/server code. Also there is
another nice thing about gdbus, it keeps us away of wrappers libraries
to use other D-Bus services because our mainloop integration is not
compatible with glib-dbus nor QDbus.

> My feeling goes the other way round: I wish we had the gdbus API *outside* BlueZ, as a library, so any C application could enjoy it

It probably easier to maintain this way, less build dependencies and
no compromise with API changes, besides iirc gdbus name has been taken
by the new GObject binding. If gnome guys could take it as part of
glib's code and build GObject binding on top of our gdbus, IMO that
would be very convenient for everybody.


-- 
Luiz Augusto von Dentz
Computer Engineer

Re: [PATCH v0 0/2] gdbus introspection fixes

[Johan Hedberg]

Hi Daniel,

On Fri, Jan 14, 2011, Daniel Wagner wrote:
> During some connman debug session I found some small hickups in the
> introspection code in gdbus. Marcel ask me to check with you first.
> 
> cheers,
> daniel
> 
> Daniel Wagner (2):
>   gdbus: invaldate_parent_data: walk the whole path down
>   gdbus: Remove root node 'name' attribute in introspection
> 
>  gdbus/object.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)

Both patches have been pushed upstream. Thanks.

Johan