Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sd_bus_add_object_manager() excludes the object on its own path #525

Closed
npmccallum opened this issue Jul 8, 2015 · 8 comments · Fixed by #532
Closed

sd_bus_add_object_manager() excludes the object on its own path #525

npmccallum opened this issue Jul 8, 2015 · 8 comments · Fixed by #532
Assignees
Labels
bug 🐛 Programming errors, that need preferential fixing sd-bus

Comments

@npmccallum
Copy link
Contributor

Here is the scenario:

sd_bus_add_object_manager(bus, NULL, "/");
sd_bus_add_object_vtable(bus, NULL, "/", "my.first.Interface", vtable, NULL);
sd_bus_add_object_vtable(bus, NULL, "/foo", "my.second.Interface", vtable, NULL);

The first call creates an ObjectManager on "/". The second and third calls create and expose an object on "/" and "/foo", respectively. The ObjectManager exposes "/foo" but NOT "/".

This behavior differs from other implementations. In particular, Bluez expects the ObjectManager on "/" to expose BOTH "/" and "/foo".

@zonque zonque added the sd-bus label Jul 8, 2015
npmccallum pushed a commit to npmccallum/systemd that referenced this issue Jul 8, 2015
Before this patch, the object manager would report on all child
objects but not the object that the object manager itself is on.
This differs from the behavior of existing DBus implementations
which also include the object itself in the object manager reports.

Fixes systemd#525
npmccallum pushed a commit to npmccallum/systemd that referenced this issue Jul 8, 2015
Before this patch, the object manager would report on all child
objects but not the object that the object manager itself is on.
This differs from the behavior of existing DBus implementations
which also include the object itself in the object manager reports.

Fixes systemd#525
@dvdhrm dvdhrm self-assigned this Jul 9, 2015
@dvdhrm dvdhrm added the bug 🐛 Programming errors, that need preferential fixing label Jul 9, 2015
dvdhrm pushed a commit to dvdhrm/systemd that referenced this issue Jul 9, 2015
If GetManagedObjects is called on /foo/bar, then it should also include
the object /foo/bar, if it exists. Right now, we only include objects
underneath /foo/bar/.

This follows the behavior of existing dbus implementations.

Obsoletes systemd#527 and fixes systemd#525. Reported by: Nathaniel McCallum
@poettering
Copy link
Member

I figure gdbus is not in sync on this with what the spec says. I have the suspicion though that the spec should be adjusted to match what gdbus does, rather than the other way round. Hence I filed this now:

https://bugs.freedesktop.org/show_bug.cgi?id=91283

@Alphix
Copy link
Contributor

Alphix commented Jul 31, 2015

That's not what I'm seeing. A simple gdbus test case with an object manager at /foo and objects at /foor/bar1 and /foo/bar2 only lists the two objects (like the spec says)

@dvdhrm
Copy link
Contributor

dvdhrm commented Jul 31, 2015

@npmccallum: Care to comment on this?

If BlueZ is the only one doing this wrong, I guess we should follow what gdbus does (and what the spec says) and revert this patch.

@npmccallum
Copy link
Contributor Author

Bluez defines the expectation here: http://git.kernel.org/cgit/bluetooth/bluez.git/tree/doc/gatt-api.txt#n266

Bluez also has working test examples here: http://git.kernel.org/cgit/bluetooth/bluez.git/tree/test/example-gatt-server

Upon further inspection, it appears they implement ObjectManager themselves incorrectly. See the offending line here: http://git.kernel.org/cgit/bluetooth/bluez.git/tree/test/example-gatt-server#n91

So I expect that this patch should be reverted and Bluez should be fixed.

@npmccallum
Copy link
Contributor Author

I have reported the issue here: http://marc.info/?l=linux-bluetooth&m=143836314007389&w=2

@poettering poettering reopened this Jul 31, 2015
@poettering
Copy link
Member

We should revert some patches in sd-bus then... reopened.

@Alphix
Copy link
Contributor

Alphix commented Jul 31, 2015

Reverting 92d16a5 seems sufficient?

@dvdhrm
Copy link
Contributor

dvdhrm commented Aug 24, 2015

Applied as f2196cf.

@dvdhrm dvdhrm closed this as completed Aug 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Programming errors, that need preferential fixing sd-bus
Development

Successfully merging a pull request may close this issue.

5 participants