Can we change org.openobex.Client's dbus node path?

Can we change org.openobex.Client's dbus node path?

[Liu, Raymond]

Hi

	I notice that, in obex-client, we create the org.openobex.Client interface under path "/".
	It's the same as org.openobex.manager in obexd.

	While, when create new session for say FTP, Obexd put Session interface under path "/session*" , and obex client put the Session interface under path "/org/openobex/Session*"

	And this will make the introspect function on client: dbus-send --print-reply --dest=org.openobex.client / org.freedesktop.DBus.Introspectable.Introspect fail to show the node for /org/openobex/Session* (but dbus-send --print-reply --dest=org.openobex.client /org org.freedesktop.DBus.Introspectable.Introspect will work)
	While for obexd: dbus-send --print-reply --dest=org.openobex / org.freedesktop.DBus.Introspectable.Introspect can show the node for /Session*.

	So, can we change the path for org.openobex.Client and register it under path "/org/openobex/Client"?  Or we register session, FTP interfaces under root path like that in the obexd? Either way can solve the introspect problem.

Best Regards,
Raymond Liu

Re: Can we change org.openobex.Client's dbus node path?

[Marcel Holtmann]

Hi Raymond,

> 	I notice that, in obex-client, we create the org.openobex.Client  
> interface under path "/".
> 	It's the same as org.openobex.manager in obexd.
>
> 	While, when create new session for say FTP, Obexd put Session  
> interface under path "/session*" , and obex client put the Session  
> interface under path "/org/openobex/Session*"
>
> 	And this will make the introspect function on client: dbus-send -- 
> print-reply --dest=org.openobex.client /  
> org.freedesktop.DBus.Introspectable.Introspect fail to show the node  
> for /org/openobex/Session* (but dbus-send --print-reply -- 
> dest=org.openobex.client /org  
> org.freedesktop.DBus.Introspectable.Introspect will work)
> 	While for obexd: dbus-send --print-reply --dest=org.openobex /  
> org.freedesktop.DBus.Introspectable.Introspect can show the node  
> for /Session*.
>
> 	So, can we change the path for org.openobex.Client and register it  
> under path "/org/openobex/Client"?  Or we register session, FTP  
> interfaces under root path like that in the obexd? Either way can  
> solve the introspect problem.

we can put the object path wherever we want. That is the whole point  
of having a manager interface to discover/create them. The API  
specification mentions "variable prefix" for a really good reason. So  
whatever is broken here, the fix is not to change the object path. We  
have to fix the broken introspection.

Also I don't see your issue here. So you are saying that within obex- 
client when you introspect / that you are not getting /org in the list  
of paths.

Regards

Marcel

RE: Can we change org.openobex.Client's dbus node path?

[Liu, Raymond]

>
>Also I don't see your issue here. So you are saying that within obex-
>client when you introspect / that you are not getting /org in the list
>of paths.
>

Yes, can not get /org in the node list for org.openobex.client

>Regards
>
>Marcel

Re: Can we change org.openobex.Client's dbus node path?

[Marcel Holtmann]

Hi Raymond,

>> Also I don't see your issue here. So you are saying that within obex-
>> client when you introspect / that you are not getting /org in the  
>> list
>> of paths.
>>
>
> Yes, can not get /org in the node list for org.openobex.client

this is a bug in gdbus then. The exactly same code is used in  
bluetoothd and connmand so it seems weird. Did you try to debug it  
where it actually fails to get this wrong? Are we missing some  
"invalidate parent" calls?

Regards

Marcel

RE: Can we change org.openobex.Client's dbus node path?

[Liu, Raymond]

>
>this is a bug in gdbus then. The exactly same code is used in
>bluetoothd and connmand so it seems weird. Did you try to debug it
>where it actually fails to get this wrong? Are we missing some
>"invalidate parent" calls?
>
>Regards
>
>Marcel

Hi Marcel

	I found the reason. During interface register object_path_unref did call i=
nvalidate_parent_data. But for our case: we have path "/" at first, and we =
register interface at "/org/openobex/Session0", there are no data for "/org=
/openobex", so no any parent introspect data is invalidated. But the whole =
/org/openobex path is created for the first time. So actually the "/"'s int=
rospect data need to be invalidated.

I modify the code as below to invalidate the parent data up to "/", so that=
 it will fix this problem.

-------------------------------------------------------------------
diff --git a/gdbus/object.c b/gdbus/object.c
old mode 100644
new mode 100755
index a417ab9..074f40b
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -299,25 +299,28 @@ static void invalidate_parent_data(DBusConnection *co=
nn, const char *child_path)
        char *parent_path, *slash;

        parent_path =3D g_strdup(child_path);
-       slash =3D strrchr(parent_path, '/');
-       if (!slash)
-               goto done;
-
-       *slash =3D '\0';
-       if (!strlen(parent_path))
-               goto done;
+       do{
+               slash =3D strrchr(parent_path, '/');
+               if (!slash)
+                       break;

-       if (!dbus_connection_get_object_path_data(conn, parent_path,
-                                                       (void *) &data))
-               goto done;
+               if (slash =3D=3D parent_path) {
+                       *(slash+1) =3D '\0';
+               } else {
+                       *slash =3D '\0';
+               }

-       if (!data)
-               goto done;
+               if (!dbus_connection_get_object_path_data(conn, parent_path=
,
+                                                               (void *) &d=
ata))
+                       continue;

-       g_free(data->introspect);
-       data->introspect =3D NULL;
+               if (!data)
+                       continue;

-done:
+               g_free(data->introspect);
+               data->introspect =3D NULL;
+       }while(slash !=3D parent_path);
+
        g_free(parent_path);
 }

-------------------------------------------------------------------------

However, It got some drawback here: if part of the parent path did exist be=
fore, and the parent's parents data do not need to change. This fix will do=
 much more thing then it should. I guess to found out all the change up to =
the necessay level will need more code. So I just make it simple here.

Raymond

RE: Can we change org.openobex.Client's dbus node path?

[Liu, Raymond]

>
>	I found the reason. During interface register object_path_unref did call
Sorry, typo here: should be object_path_ref

>invalidate_parent_data. But for our case: we have path "/" at first, and w=
e
>register interface at "/org/openobex/Session0", there are no data for
>"/org/openobex", so no any parent introspect data is invalidated. But the =
whole
>/org/openobex path is created for the first time. So actually the "/"'s
>introspect data need to be invalidated.

Re: Can we change org.openobex.Client's dbus node path?

[Marcel Holtmann]

Hi Raymond,

>> this is a bug in gdbus then. The exactly same code is used in
>> bluetoothd and connmand so it seems weird. Did you try to debug it
>> where it actually fails to get this wrong? Are we missing some
>> "invalidate parent" calls?
>>
> 	I found the reason. During interface register object_path_unref did  
> call invalidate_parent_data. But for our case: we have path "/" at  
> first, and we register interface at "/org/openobex/Session0", there  
> are no data for "/org/openobex", so no any parent introspect data is  
> invalidated. But the whole /org/openobex path is created for the  
> first time. So actually the "/"'s introspect data need to be  
> invalidated.

I suspected that this might be the reason. I fixed a similar bug some  
time ago. Funny that BlueZ didn't run into this since it might suffer  
the same with the new modified adapter and device object paths.

> I modify the code as below to invalidate the parent data up to "/",  
> so that it will fix this problem.
>
> -------------------------------------------------------------------
> diff --git a/gdbus/object.c b/gdbus/object.c
> old mode 100644
> new mode 100755
> index a417ab9..074f40b
> --- a/gdbus/object.c
> +++ b/gdbus/object.c
> @@ -299,25 +299,28 @@ static void  
> invalidate_parent_data(DBusConnection *conn, const char *child_path)
>        char *parent_path, *slash;
>
>        parent_path = g_strdup(child_path);
> -       slash = strrchr(parent_path, '/');
> -       if (!slash)
> -               goto done;
> -
> -       *slash = '\0';
> -       if (!strlen(parent_path))
> -               goto done;
> +       do{
> +               slash = strrchr(parent_path, '/');
> +               if (!slash)
> +                       break;
>
> -       if (!dbus_connection_get_object_path_data(conn, parent_path,
> -                                                       (void *)  
> &data))
> -               goto done;
> +               if (slash == parent_path) {
> +                       *(slash+1) = '\0';
> +               } else {
> +                       *slash = '\0';
> +               }
>
> -       if (!data)
> -               goto done;
> +               if (!dbus_connection_get_object_path_data(conn,  
> parent_path,
> +                                                                
> (void *) &data))
> +                       continue;
>
> -       g_free(data->introspect);
> -       data->introspect = NULL;
> +               if (!data)
> +                       continue;
>
> -done:
> +               g_free(data->introspect);
> +               data->introspect = NULL;
> +       }while(slash != parent_path);
> +
>        g_free(parent_path);
> }
>
> -------------------------------------------------------------------------
>
> However, It got some drawback here: if part of the parent path did  
> exist before, and the parent's parents data do not need to change.  
> This fix will do much more thing then it should. I guess to found  
> out all the change up to the necessay level will need more code. So  
> I just make it simple here.

so first, please fix the coding style here. I can tell from just the  
pure email that you are mixing up tabs and spaces and also that you  
are missing whitespaces here and there. Check that your mail client  
doesn't mangle it. If in doubt send it as attachment.

Regards

Marcel

RE: Can we change org.openobex.Client's dbus node path?

[Liu, Raymond]

>
>so first, please fix the coding style here. I can tell from just the
>pure email that you are mixing up tabs and spaces and also that you
>are missing whitespaces here and there. Check that your mail client
>doesn't mangle it. If in doubt send it as attachment.
>
>Regards
>
>Marcel


Hi Marcel

Thanks for point out the tabs and spaces issue, I think this is might be re=
lated to my terminal setting when I do the copy/paste with the diff output.=
 And 3 whitespaces added. So here it is:

---------------------------------------------------------------------------

diff --git a/gdbus/object.c b/gdbus/object.c
index a417ab9..9cf6f09 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -299,25 +299,28 @@ static void invalidate_parent_data(DBusConnection *co=
nn, const char *child_path)
 	char *parent_path, *slash;
=20
 	parent_path =3D g_strdup(child_path);
-	slash =3D strrchr(parent_path, '/');
-	if (!slash)
-		goto done;
-
-	*slash =3D '\0';
-	if (!strlen(parent_path))
-		goto done;
+	do {
+		slash =3D strrchr(parent_path, '/');
+		if (!slash)
+			break;
=20
-	if (!dbus_connection_get_object_path_data(conn, parent_path,
-							(void *) &data))
-		goto done;
+		if (slash =3D=3D parent_path) {
+			*(slash + 1) =3D '\0';
+		} else {
+			*slash =3D '\0';
+		}
=20
-	if (!data)
-		goto done;
+		if (!dbus_connection_get_object_path_data(conn, parent_path,
+								(void *) &data))
+			continue;
=20
-	g_free(data->introspect);
-	data->introspect =3D NULL;
+		if (!data)
+			continue;
=20
-done:
+		g_free(data->introspect);
+		data->introspect =3D NULL;
+	} while (slash !=3D parent_path);
+=09
 	g_free(parent_path);
 }

---------------------------------------------

Raymond