From a7bbff5baf1a9b9c64d7b92bfbef5406ebbdd710 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 11 Oct 2022 18:36:00 +0100 Subject: [PATCH 1/2] test-autolaunch-win: Don't overwrite an error with another error An unrelated branch failed CI with this assertion failure: 26: dbus[6768]: error: arguments to dbus_set_error() were incorrect, assertion "(error) == NULL || !dbus_error_is_set ((error))" failed in file ...\dbus-errors.c line 365. Looking at the test, this seems to be the most likely candidate for this bug in error handling, which is masking whatever the real cause for the failure was (we can't tell from here). If dbus_connection_send_with_reply_and_block() returns NULL, then it should already have set the error. Fixing this bug in the error handling will hopefully give us a better error message for the actual failure if it happens again. Signed-off-by: Simon McVittie --- test/name-test/test-autolaunch-win.c | 1 - 1 file changed, 1 deletion(-) diff --git a/test/name-test/test-autolaunch-win.c b/test/name-test/test-autolaunch-win.c index 8a434e2c..e5e0da7f 100644 --- a/test/name-test/test-autolaunch-win.c +++ b/test/name-test/test-autolaunch-win.c @@ -71,7 +71,6 @@ call_method (DBusConnection *conn, dbus_message_unref (method); if (reply == NULL) { - dbus_set_error (error, DBUS_ERROR_FAILED, "Got no reply"); result = FALSE; goto out; } From f325252e5ee1316b996affe35006289d1324aa2a Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 11 Oct 2022 18:41:00 +0100 Subject: [PATCH 2/2] test-autolaunch-win: Remove redundant check for ERROR message This seems to have been intended to give a more specific error message if the method call failed, but it will not have been effective, because dbus_connection_send_with_reply_and_block() ends with a check for ERROR messages using dbus_set_error_from_message(). This means that if the reply was an ERROR message, it will already have been converted into a DBusError by the time call_method() regains control. Signed-off-by: Simon McVittie --- test/name-test/test-autolaunch-win.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/test/name-test/test-autolaunch-win.c b/test/name-test/test-autolaunch-win.c index e5e0da7f..30b8353b 100644 --- a/test/name-test/test-autolaunch-win.c +++ b/test/name-test/test-autolaunch-win.c @@ -34,21 +34,6 @@ static int add_wait_time = 0; #define oom() _dbus_test_fatal ("Out of memory") -/** - * helper function - */ -#define _dbus_error_set_from_message_with_location(a, b) __dbus_error_set_from_message_with_location (__FILE__, __LINE__, __FUNCTION__, a, b) - -static void -__dbus_error_set_from_message_with_location (const char *file, int line, const char *function, DBusError *error, DBusMessage *message) -{ - char *str = NULL; - dbus_message_get_args (message, NULL, - DBUS_TYPE_STRING, &str, - DBUS_TYPE_INVALID); - dbus_set_error (error, dbus_message_get_error_name (message), "[%s(%d):%s] %s", file, line, function, str ? str : ""); -} - static dbus_bool_t call_method (DBusConnection *conn, DBusError *error, @@ -75,12 +60,8 @@ call_method (DBusConnection *conn, goto out; } - if (dbus_message_get_type (reply) == DBUS_MESSAGE_TYPE_ERROR) - { - _dbus_error_set_from_message_with_location (error, reply); - result = FALSE; - goto out; - } + /* ..._send_with_reply_and_block converts ERROR messages into errors */ + _dbus_assert (dbus_message_get_type (reply) != DBUS_MESSAGE_TYPE_ERROR); result = TRUE; out: