Make _dbus_get_local_machine_uuid_encoded() properly failable

This function already raised an error, and all callers handled that
error as gracefully as they could (because _dbus_generate_uuid() is
failable, since 2015). Given that, it seems unnecessarily hostile
to do a _dbus_warn_check_failed() unless we have no better alternative:
yes, it indicates that dbus has not been installed correctly, but
during build-time tests it's entirely reasonable that dbus has not
yet been installed.

Callers are:

* DBusConnection, to implement Peer.GetMachineId()
* The bus driver, to implement Peer.GetMachineId()
* X11 autolaunching
* dbus_get_local_machine_id()

Of those, only the last one is not in a position to return an error
gracefully, so move the _dbus_warn_check_failed() to there.

Migrate the text about the D-Bus library being incorrectly set up
into the error emitted by the Unix implementation, and to make it
less misleading, include separate error messages for both the
files we try to read:

$ bwrap --ro-bind / / --dev /dev --tmpfs /etc --tmpfs /var \
  ./tools/dbus-uuidgen --get
D-Bus library appears to be incorrectly set up: see the manual
page for dbus-uuidgen to correct this issue. (Failed to open
"/var/lib/dbus/machine-id": No such file or directory; Failed to open
"/etc/machine-id": No such file or directory)

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=13194

diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c
index 1fb6f02..bc3454d 100644 (file)
--- a/dbus/dbus-internals.c
+++ b/dbus/dbus-internals.c
@@ -908,25 +908,8 @@ _dbus_get_local_machine_uuid_encoded (DBusString *uuid_str,
 
   if (machine_uuid_initialized_generation != _dbus_current_generation)
     {
-      DBusError local_error = DBUS_ERROR_INIT;
-
-      if (!_dbus_read_local_machine_uuid (&machine_uuid, FALSE,
-                                          &local_error))
-        {          
-#ifndef DBUS_ENABLE_EMBEDDED_TESTS
-          /* For the test suite, we may not be installed so just continue silently
-           * here. But in a production build, we want to be nice and loud about
-           * this.
-           */
-          _dbus_warn_check_failed ("D-Bus library appears to be incorrectly set up; failed to read machine uuid: %s\n"
-                                   "See the manual page for dbus-uuidgen to correct this issue.\n",
-                                   local_error.message);
-#endif
-          
-          dbus_error_free (&local_error);
-          
-          ok = _dbus_generate_uuid (&machine_uuid, error);
-        }
+      if (!_dbus_read_local_machine_uuid (&machine_uuid, FALSE, error))
+        ok = FALSE;
     }
 
   if (ok)

diff --git a/dbus/dbus-misc.c b/dbus/dbus-misc.c
index a2da562..851fa2b 100644 (file)
--- a/dbus/dbus-misc.c
+++ b/dbus/dbus-misc.c
@@ -67,11 +67,17 @@
  * The UUID is not a UUID in the sense of RFC4122; the details
  * are explained in the D-Bus specification.
  *
- * @returns a 32-byte-long hex-encoded UUID string, or #NULL if insufficient memory
+ * This function returns #NULL if there was not enough memory to read
+ * the UUID, or if the UUID could not be read because the D-Bus
+ * library was installed incorrectly. In the latter case, a warning
+ * is logged.
+ *
+ * @returns a 32-byte-long hex-encoded UUID string, or #NULL on failure
  */
 char*
 dbus_get_local_machine_id (void)
 {
+  DBusError error = DBUS_ERROR_INIT;
   DBusString uuid;
   char *s;
 
@@ -82,10 +88,20 @@ dbus_get_local_machine_id (void)
 
   /* The documentation says dbus_get_local_machine_id() only fails on OOM;
    * this can actually also fail if the D-Bus installation is faulty
-   * (no UUID) *and* reading a new random UUID fails, but we have no way
-   * to report that */
-  if (!_dbus_get_local_machine_uuid_encoded (&uuid, NULL) ||
-      !_dbus_string_steal_data (&uuid, &s))
+   * (no UUID), but we have no good way to report that. Historically,
+   * _dbus_get_local_machine_uuid_encoded was responsible for issuing the
+   * warning; now we do that here. */
+  if (!_dbus_get_local_machine_uuid_encoded (&uuid, &error))
+    {
+      if (!dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
+        _dbus_warn_check_failed ("%s", error.message);
+
+      _dbus_string_free (&uuid);
+      dbus_error_free (&error);
+      return NULL;
+    }
+
+  if (!_dbus_string_steal_data (&uuid, &s))
     {
       _dbus_string_free (&uuid);
       return NULL;

diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
index 7bfb66f..92217e3 100644 (file)
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -3905,20 +3905,20 @@ _dbus_read_local_machine_uuid (DBusGUID   *machine_id,
                                dbus_bool_t create_if_not_found,
                                DBusError  *error)
 {
+  DBusError our_error = DBUS_ERROR_INIT;
+  DBusError etc_error = DBUS_ERROR_INIT;
   DBusString filename;
   dbus_bool_t b;
 
   _dbus_string_init_const (&filename, DBUS_MACHINE_UUID_FILE);
 
-  b = _dbus_read_uuid_file (&filename, machine_id, FALSE, error);
+  b = _dbus_read_uuid_file (&filename, machine_id, FALSE, &our_error);
   if (b)
     return TRUE;
 
-  dbus_error_free (error);
-
   /* Fallback to the system machine ID */
   _dbus_string_init_const (&filename, "/etc/machine-id");
-  b = _dbus_read_uuid_file (&filename, machine_id, FALSE, error);
+  b = _dbus_read_uuid_file (&filename, machine_id, FALSE, &etc_error);
 
   if (b)
     {
@@ -3930,14 +3930,26 @@ _dbus_read_local_machine_uuid (DBusGUID   *machine_id,
           _dbus_write_uuid_file (&filename, machine_id, NULL);
         }
 
+      dbus_error_free (&our_error);
       return TRUE;
     }
 
   if (!create_if_not_found)
-    return FALSE;
+    {
+      dbus_set_error (error, etc_error.name,
+                      "D-Bus library appears to be incorrectly set up: "
+                      "see the manual page for dbus-uuidgen to correct "
+                      "this issue. (%s; %s)",
+                      our_error.message, etc_error.message);
+      dbus_error_free (&our_error);
+      dbus_error_free (&etc_error);
+      return FALSE;
+    }
+
+  dbus_error_free (&our_error);
+  dbus_error_free (&etc_error);
 
   /* if none found, try to make a new one */
-  dbus_error_free (error);
   _dbus_string_init_const (&filename, DBUS_MACHINE_UUID_FILE);
 
   if (!_dbus_generate_uuid (machine_id, error))