From 41bf94a81d2b8b974bb5889b5e8ad8e7721545c0 Mon Sep 17 00:00:00 2001 From: Simon McVittie 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 --- 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