After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 663928 - Missing fallback code for /etc/machine-id
Missing fallback code for /etc/machine-id
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gdbus
unspecified
Other Linux
: Normal major
: ---
Assigned To: David Zeuthen (not reading bugmail)
gtkdev
Depends on:
Blocks:
 
 
Reported: 2011-11-12 17:30 UTC by Samuli Suominen
Modified: 2011-11-18 04:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (1.05 KB, patch)
2011-11-14 07:44 UTC, Alexandre Rostovtsev
needs-work Details | Review
proposed patch (1.48 KB, patch)
2011-11-15 09:18 UTC, Alexandre Rostovtsev
needs-work Details | Review
_g_dbus_get_machine_id(): check /etc/machine-id too (1.58 KB, patch)
2011-11-16 15:24 UTC, Simon McVittie
accepted-commit_now Details | Review
_g_dbus_get_machine_id(): check /etc/machine-id too (1.58 KB, patch)
2011-11-18 04:10 UTC, Matthias Clasen
committed Details | Review

Description Samuli Suominen 2011-11-12 17:30:11 UTC
gio/gdbusprivate.c has hardcoded path to /var/lib/dbus/machine-id, while it's legal to not have the file at all and have /etc/machine-id instead, for systemd compability

the relavent commit is:

http://cgit.freedesktop.org/dbus/dbus/commit/dbus/dbus-sysdeps-unix.c?id=66e52541d5bdd4927a5c702963749760643313f4

very ugly workaround:

sed -i -e '/g_file_get_contents/s:/var/lib/dbus/machine-id:/etc/machine-id:' gio/gdbusprivate.c

downstream bug:

http://bugs.gentoo.org/390143
Comment 1 Alexandre Rostovtsev 2011-11-14 07:44:28 UTC
Created attachment 201347 [details] [review]
proposed patch
Comment 2 Arun Raghavan 2011-11-14 08:06:01 UTC
Looking at how this is done in D-Bus, it appears the order you have is inverted: http://cgit.freedesktop.org/dbus/dbus/commit/dbus/dbus-sysdeps-unix.c?id=66e52541d5bdd4927a5c702963749760643313f4
Comment 3 David Zeuthen (not reading bugmail) 2011-11-14 15:22:42 UTC
Review of attachment 201347 [details] [review]:

::: gio/gdbusprivate.c
@@ +1993,3 @@
                             &ret,
                             NULL,
                             error))

First of all, as comment 2 says, you got the check inverted. Second, you need to adjust the error, e.g.

  g_prefix_error (error, _("Unable to load /var/lib/dbus/machine-id: "));

so it matches the proper location.
Comment 4 David Zeuthen (not reading bugmail) 2011-11-14 15:23:08 UTC
Please use gdbus, not gio for GDBus bugs (otherwise I won't see them).
Comment 5 Alexandre Rostovtsev 2011-11-15 09:18:43 UTC
Created attachment 201426 [details] [review]
proposed patch

(In reply to comment #3)
> First of all, as comment 2 says, you got the check inverted. Second, you need
> to adjust the error, e.g.
> 
>   g_prefix_error (error, _("Unable to load /var/lib/dbus/machine-id: "));
> 
> so it matches the proper location.

OK, here's a second attempt. A temporary GError has to be used because g_file_get_contents("/var/lib/dbus/machine-id",...)'s error should only be propagated if g_file_get_contents("/etc/machine-id",...) also fails. And g_file_get_contents("/etc/machine-id",...) does not save its error because comment #2 implies that /var/lib/dbus is the default location for machine-id, and there doesn't seem to be a clean, translation-friendly way of combining two GErrors.
Comment 6 Simon McVittie 2011-11-16 14:38:31 UTC
Review of attachment 201426 [details] [review]:

I'm not a GLib/GDBus reviewer, but I can see one problem with this:

::: gio/gdbusprivate.c
@@ +2003,3 @@
     {
       /* TODO: validate value */
+      g_error_free (first_error);

This will issue a critical warning (or perhaps just segfault, depending how GLib was compiled) if /var/lib/dbus/machine-id was read successfully (i.e. first_error is still NULL). Use g_clear_error (&first_error) instead.

With that change, I'd be happy with this patch (but again, I'm not an official reviewer).

I'll do some tests in chroots with each of the files, neither, and both.
Comment 7 Simon McVittie 2011-11-16 15:24:35 UTC
Created attachment 201548 [details] [review]
_g_dbus_get_machine_id(): check /etc/machine-id too

machine-id can be in /etc or in /var/lib/dbus.

[amended with slightly revised error handling -smcv]

---

I've attributed this patch to Alexandre.

I tested it in a chroot environment with a test program that just calls dbus_bus_get_sync():

# /etc/machine-id only
reptile(sid-amd64)% sudo install ~/tmp/uuid /etc/machine-id
reptile(sid-amd64)% LD_LIBRARY_PATH=gio/.libs:glib/.libs:gobject/.libs ~/tmp/a.out
could not connect: Error spawning command line `dbus-launch --autolaunch=804eeb4e85cd6eb299ad6675000068c1 --binary-syntax --close-stderr': Failed to execute child process "dbus-launch" (No such file or directory)
# this indicates that the machine-id was read correctly: my chroot
# doesn't have dbus-launch

# both locations
reptile(sid-amd64)% sudo install -d /var/lib/dbus
reptile(sid-amd64)% sudo cp /etc/machine-id /var/lib/dbus/machine-id
reptile(sid-amd64)% LD_LIBRARY_PATH=gio/.libs:glib/.libs:gobject/.libs ~/tmp/a.out
could not connect: Error spawning command line `dbus-launch --autolaunch=804eeb4e85cd6eb299ad6675000068c1 --binary-syntax --close-stderr': Failed to execute child process "dbus-launch" (No such file or directory)

# traditional location only
reptile(sid-amd64)% sudo rm /etc/machine-id
reptile(sid-amd64)% LD_LIBRARY_PATH=gio/.libs:glib/.libs:gobject/.libs ~/tmp/a.out
could not connect: Error spawning command line `dbus-launch --autolaunch=804eeb4e85cd6eb299ad6675000068c1 --binary-syntax --close-stderr': Failed to execute child process "dbus-launch" (No such file or directory)

# neither location
reptile(sid-amd64)% sudo rm /var/lib/dbus/machine-id 
reptile(sid-amd64)% LD_LIBRARY_PATH=gio/.libs:glib/.libs:gobject/.libs ~/tmp/a.out
could not connect: Cannot spawn a message bus without a machine-id: Unable to load /var/lib/dbus/machine-id or /etc/machine-id: Failed to open file '/var/lib/dbus/machine-id': No such file or directory
Comment 8 David Zeuthen (not reading bugmail) 2011-11-16 15:42:40 UTC
Comment on attachment 201548 [details] [review]
_g_dbus_get_machine_id(): check /etc/machine-id too

Looks good to me. Thanks Alexandre and Simon!
Comment 9 Matthias Clasen 2011-11-18 04:10:26 UTC
The following fix has been pushed:
dc89b51 _g_dbus_get_machine_id(): check /etc/machine-id too
Comment 10 Matthias Clasen 2011-11-18 04:10:30 UTC
Created attachment 201640 [details] [review]
_g_dbus_get_machine_id(): check /etc/machine-id too

machine-id can be in /etc or in /var/lib/dbus.

[amended with slightly revised error handling -smcv]

Bug: