Attachment 131784 Details for Bug 13194 – [2/3] Make _dbus_get_local_machine_uuid_encoded() properly failable

[patch] [2/3] Make _dbus_get_local_machine_uuid_encoded() properly failable

0013-Make-_dbus_get_local_machine_uuid_encoded-properly-f.patch (text/plain), 6.61 KB, created by Simon McVittie on 2017-06-07 17:08:42 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2017-06-07 17:08:42 UTC

Size: 6.61 KB

patch

obsolete

>From 41bf94a81d2b8b974bb5889b5e8ad8e7721545c0 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <smcv@collabora.com>
>Date: Wed, 7 Jun 2017 17:14:00 +0100
>Subject: [PATCH 13/14] 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>
>---
> dbus/dbus-internals.c    | 21 ++-------------------
> dbus/dbus-misc.c         | 26 +++++++++++++++++++++-----
> dbus/dbus-sysdeps-unix.c | 24 ++++++++++++++++++------
> 3 files changed, 41 insertions(+), 30 deletions(-)
>
>diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c
>index 1fb6f02e..bc3454d5 100644
>--- 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 a2da562a..851fa2b1 100644
>--- 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 7bfb66fe..92217e35 100644
>--- 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))
>-- 
>2.11.0
>

Actions: View | Splinter Review

Attachments on bug 13194: 131783 | 131784 | 131785 | 131809 | 131813

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.