From d47154c28b68b32177773bd489fdae62b440f38c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 16 Mar 2010 22:25:40 -0400 Subject: [PATCH 1/9] Delete name-test/test-names This test, while extensive, has the serious flaw of effectively spinning on _dbus_connection_do_iteration_unlocked. Any tests like this should be using the internal DBus mainloop, which I don't have time to port it to before doing a release. --- test/name-test/Makefile.am | 8 +- test/name-test/run-test.sh | 3 - test/name-test/test-names.c | 565 ------------------------------------ 3 files changed, 1 insertion(+), 575 deletions(-) delete mode 100644 test/name-test/test-names.c diff --git a/test/name-test/Makefile.am b/test/name-test/Makefile.am index d8e72d14..9a508a12 100644 --- a/test/name-test/Makefile.am +++ b/test/name-test/Makefile.am @@ -16,13 +16,7 @@ if DBUS_BUILD_TESTS ## we use noinst_PROGRAMS not check_PROGRAMS for TESTS so that we ## build even when not doing "make check" -noinst_PROGRAMS=test-names test-pending-call-dispatch test-pending-call-timeout test-threads-init test-ids test-shutdown test-privserver test-privserver-client - -test_names_SOURCES= \ - test-names.c - -test_names_LDADD=$(top_builddir)/dbus/libdbus-convenience.la $(DBUS_TEST_LIBS) -test_names_LDFLAGS=@R_DYNAMIC_LDFLAG@ +noinst_PROGRAMS=test-pending-call-dispatch test-pending-call-timeout test-threads-init test-ids test-shutdown test-privserver test-privserver-client test_pending_call_dispatch_SOURCES = \ test-pending-call-dispatch.c diff --git a/test/name-test/run-test.sh b/test/name-test/run-test.sh index 4eb24252..832ce0a5 100755 --- a/test/name-test/run-test.sh +++ b/test/name-test/run-test.sh @@ -33,9 +33,6 @@ fi echo "running test-ids" ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/name-test/test-ids || die "test-ids failed" -echo "running test-names" -${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/name-test/test-names || die "test-names failed" - echo "running test-pending-call-dispatch" ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/name-test/test-pending-call-dispatch || die "test-pending-call-dispatch failed" diff --git a/test/name-test/test-names.c b/test/name-test/test-names.c deleted file mode 100644 index b09f3638..00000000 --- a/test/name-test/test-names.c +++ /dev/null @@ -1,565 +0,0 @@ -#include -#include -#include -#include -#include -#ifdef HAVE_UNISTD_H -#include -#endif - -#define REMOVE_CONNECTION 0 -#define ADD_CONNECTION 1 -#define ALLOW_REPLACEMENT DBUS_NAME_FLAG_ALLOW_REPLACEMENT -#define REPLACE_EXISTING DBUS_NAME_FLAG_REPLACE_EXISTING -#define DO_NOT_QUEUE DBUS_NAME_FLAG_DO_NOT_QUEUE - -#define PRIMARY_OWNER DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER -#define IN_QUEUE DBUS_REQUEST_NAME_REPLY_IN_QUEUE -#define EXISTS DBUS_REQUEST_NAME_REPLY_EXISTS -#define ALREADY_OWNER DBUS_REQUEST_NAME_REPLY_ALREADY_OWNER - -#define RELEASED DBUS_RELEASE_NAME_REPLY_RELEASED -#define NON_EXISTANT DBUS_RELEASE_NAME_REPLY_NON_EXISTENT -#define NOT_OWNER DBUS_RELEASE_NAME_REPLY_NOT_OWNER - -#define NUM_CONN 4 -#define TEST_NAME "org.freedesktop.DBus.TestSuite.NameTest" -#define NUM_TRIES_TIL_FAIL 15 - -typedef struct { - int command; - - int connection_number; - dbus_uint32_t flags; - - dbus_uint32_t expected_result; - - int expected_queue[NUM_CONN]; -} CommandAndResult; - -static CommandAndResult test_data[] = { - {ADD_CONNECTION, 0, ALLOW_REPLACEMENT | REPLACE_EXISTING, - PRIMARY_OWNER, {0,-1,-1,-1}}, - {ADD_CONNECTION, 0, REPLACE_EXISTING, - ALREADY_OWNER, {0,-1,-1,-1}}, - {ADD_CONNECTION, 1, ALLOW_REPLACEMENT | REPLACE_EXISTING, - IN_QUEUE, {0,1,-1,-1}}, - {REMOVE_CONNECTION, 0, 0, - RELEASED, {1,-1,-1,-1}}, - {ADD_CONNECTION, 0, REPLACE_EXISTING | DO_NOT_QUEUE, - PRIMARY_OWNER, {0,1,-1,-1}}, - {ADD_CONNECTION, 2, ALLOW_REPLACEMENT, - IN_QUEUE, {0,1,2,-1}}, - {ADD_CONNECTION, 2, ALLOW_REPLACEMENT | REPLACE_EXISTING, - IN_QUEUE, {0,2,1,-1}}, - {ADD_CONNECTION, 0, ALLOW_REPLACEMENT | DO_NOT_QUEUE, - ALREADY_OWNER, {0,2,1,-1}}, - {ADD_CONNECTION, 1, ALLOW_REPLACEMENT | REPLACE_EXISTING, - PRIMARY_OWNER, {1,2,-1,-1}}, - {ADD_CONNECTION, 0, REPLACE_EXISTING, - PRIMARY_OWNER, {0,1,2,-1}}, - {ADD_CONNECTION, 2, DO_NOT_QUEUE, - EXISTS, {0,1,-1,-1}}, - {REMOVE_CONNECTION, 2, 0, - NOT_OWNER, {0,1,-1,-1}}, - {ADD_CONNECTION, 3, 0, - IN_QUEUE, {0,1,3,-1}}, - {ADD_CONNECTION, 0, ALLOW_REPLACEMENT, - ALREADY_OWNER, {0,1,3,-1}}, - {ADD_CONNECTION, 2, ALLOW_REPLACEMENT, - IN_QUEUE, {0,1,3,2}} -}; - -static dbus_bool_t -check_connection (DBusConnection *conn, - int iteration, - DBusConnection *uniq_conn[NUM_CONN]) -{ - DBusMessage *reply; - DBusMessage *method; - DBusError error; - char **list; - int len, i; - const char *name; - - reply = NULL; - method = NULL; - list = NULL; - - dbus_error_init (&error); - - name = TEST_NAME; - method = dbus_message_new_method_call (DBUS_SERVICE_DBUS, - DBUS_PATH_DBUS, - DBUS_INTERFACE_DBUS, - "ListQueuedOwners"); - - if (method == NULL) - goto out; - - if (!dbus_message_append_args (method, - DBUS_TYPE_STRING, &name, - DBUS_TYPE_INVALID)) - { - fprintf (stderr, "Error appending args\n") ; - goto out; - } - - reply = dbus_connection_send_with_reply_and_block (conn, - method, - -1, - &error); - - if (reply == NULL) - { - fprintf (stderr, "Error calling ListQueuedOwners: %s\n", error.message); - dbus_error_free (&error); - goto out; - } - - - - if (!dbus_message_get_args (reply, - &error, - DBUS_TYPE_ARRAY, DBUS_TYPE_STRING, - &list, &len, - DBUS_TYPE_INVALID)) - { - fprintf (stderr, "Error getting args: %s\n", error.message); - dbus_error_free (&error); - goto out; - } - - printf ("Iteration %i: ", iteration); - - if (len > NUM_CONN) - { - fprintf (stderr, "There are %i connections in the queue," - " we are only expecting up to %i connections!\n", - len, - NUM_CONN); - goto out; - } - - for (i = 0; i < len; i++) - { - int expected_conn_num; - const char *expected_uname; - - if (i > 0) - printf (", "); - - printf ("%s", list[i]); - - expected_conn_num = test_data[iteration].expected_queue[i]; - - if (expected_conn_num == -1) - { - fprintf (stderr, - "\nDid not expect this last connection" - " to be in the queue!\n"); - goto out; - } - - expected_uname = - dbus_bus_get_unique_name (uniq_conn[expected_conn_num]); - - if (strcmp (list[i], expected_uname) != 0) - { - fprintf (stderr, - "\n%s expected but %s is in the queue!\n", - expected_uname, - list[i]); - - goto out; - } - } - - printf ("\n"); - - dbus_message_unref (method); - dbus_message_unref (reply); - dbus_free_string_array (list); - return TRUE; - - out: - if (method != NULL) - dbus_message_unref (method); - - if (reply != NULL) - dbus_message_unref (reply); - - if (list != NULL) - dbus_free_string_array (list); - - return FALSE; -} - -static dbus_bool_t -match_acquired_or_lost_signal (DBusConnection *conn, const char *member, const char *name) -{ - int tries; - DBusMessage *msg; - const char *interface = "org.freedesktop.DBus"; - - for (tries = 0; tries < NUM_TRIES_TIL_FAIL; tries++) - { - _dbus_connection_lock (conn); - _dbus_connection_do_iteration_unlocked (conn, - DBUS_ITERATION_DO_READING | - DBUS_ITERATION_DO_WRITING | - DBUS_ITERATION_BLOCK, - 0); - _dbus_connection_unlock (conn); - msg = dbus_connection_pop_message (conn); - if (msg != NULL) - { - if (dbus_message_is_signal (msg, - interface, - member)) - { - const char *n; - DBusError error; - dbus_error_init (&error); - - dbus_message_get_args (msg, &error, DBUS_TYPE_STRING, &n, DBUS_TYPE_INVALID); - - if (dbus_error_is_set (&error)) - { - fprintf (stderr, "Error getting args: %s\n", error.message); - dbus_error_free (&error); - dbus_message_unref (msg); - return FALSE; - } - - if (strcmp (n, name) == 0) - { - dbus_message_unref (msg); - break; - } - } - dbus_message_unref (msg); - } - } - - if (tries == NUM_TRIES_TIL_FAIL) - { - fprintf (stderr, "Did not receive the expected %s.%s signal!!!\n", interface, member); - return FALSE; - } - - return TRUE; -} - -static dbus_bool_t -match_name_owner_changed_signal (DBusConnection *conn, - const char *bus_name, - const char *lost_name, - const char *acquired_name) -{ - int tries; - DBusMessage *msg; - - for (tries = 0; tries < NUM_TRIES_TIL_FAIL; tries++) - { - _dbus_connection_lock (conn); - _dbus_connection_do_iteration_unlocked (conn, - DBUS_ITERATION_DO_READING | - DBUS_ITERATION_DO_WRITING | - DBUS_ITERATION_BLOCK, - 0); - _dbus_connection_unlock (conn); - msg = dbus_connection_pop_message (conn); - - if (msg != NULL) - { - if (dbus_message_is_signal (msg, - "org.freedesktop.DBus", - "NameOwnerChanged")) - { - const char *n; - const char *ln; - const char *an; - DBusError error; - dbus_error_init (&error); - - dbus_message_get_args (msg, &error, DBUS_TYPE_STRING, &n, DBUS_TYPE_STRING, &ln, DBUS_TYPE_STRING, &an, DBUS_TYPE_INVALID); - - if (dbus_error_is_set (&error)) - { - fprintf (stderr, "Error getting args: %s\n", error.message); - dbus_error_free (&error); - dbus_message_unref (msg); - return FALSE; - } - - if (strcmp (n, bus_name) == 0) - { - if ((lost_name == NULL && strcmp (ln, "") == 0) - || strcmp (lost_name, ln) == 0) - { - if ((acquired_name == NULL && strcmp (an, "") == 0) - || strcmp (acquired_name, an) == 0) - { - dbus_message_unref (msg); - break; - } - else - { - fprintf (stderr, "Error: name %s was expected to be acquired but we got %s instead\n", acquired_name, an); - dbus_message_unref (msg); - return FALSE; - } - } - else - { - fprintf (stderr, "Error: name %s was expected to be lost but we got %s instead\n", lost_name, ln); - dbus_message_unref (msg); - return FALSE; - } - } - } - dbus_message_unref (msg); - } - } - - if (tries == NUM_TRIES_TIL_FAIL) - { - fprintf (stderr, "Did not receive the expected NameOwnerChanged signal, bus_name %s lost_name %s acquired_name %s\n", - bus_name ? bus_name : "(null)", - lost_name ? lost_name : "(null)", - acquired_name ? acquired_name : "(null)"); - return FALSE; - } - - return TRUE; -} - - -static dbus_bool_t -check_signals (DBusConnection *monitor, - int iteration, - DBusConnection *conn[NUM_CONN]) -{ - DBusConnection *lost_conn = NULL; - DBusConnection *acquired_conn = NULL; - const char *lost_name; - const char *acquired_name; - - if (iteration == 0) - { - int i; - i = test_data[iteration].expected_queue[0]; - - if (i >= 0) - acquired_conn = conn[i]; - } - else - { - int i; - i = test_data[iteration - 1].expected_queue[0]; - - if (i >= 0) - lost_conn = conn[i]; - - i = test_data[iteration].expected_queue[0]; - - if (i >= 0) - acquired_conn = conn[i]; - - if (acquired_conn == lost_conn) - acquired_conn = lost_conn = NULL; - } - - lost_name = lost_conn == NULL? NULL : - dbus_bus_get_unique_name (lost_conn); - - acquired_name = acquired_conn == NULL? NULL : - dbus_bus_get_unique_name (acquired_conn); - - if (lost_name != NULL) - if (!match_acquired_or_lost_signal (lost_conn, - "NameLost", - TEST_NAME)) - return FALSE; - - if (acquired_name != NULL) - if (!match_acquired_or_lost_signal (acquired_conn, - "NameAcquired", - TEST_NAME)) - return FALSE; - - if (acquired_name != NULL || lost_name != NULL) - if (!match_name_owner_changed_signal (monitor, - TEST_NAME, - lost_name, - acquired_name)) - return FALSE; - - return TRUE; -} - -int -main (int argc, char *argv[]) -{ - DBusConnection *conn[NUM_CONN]; - DBusConnection *monitor; - DBusError error; - int i; - int test_data_len; - - test_data_len = sizeof (test_data) / sizeof (CommandAndResult); - - dbus_error_init (&error); - - conn[0] = dbus_bus_get_private (DBUS_BUS_SESSION, &error); - if (dbus_error_is_set (&error)) - { - fprintf (stderr, "*** Failed to open connection 0 to session bus: %s\n", - error.message); - dbus_error_free (&error); - return 1; - } - - if (!match_acquired_or_lost_signal (conn[0], - "NameAcquired", - dbus_bus_get_unique_name (conn[0]))) - return 1; - - conn[1] = dbus_bus_get_private (DBUS_BUS_SESSION, &error); - if (dbus_error_is_set (&error)) - { - fprintf (stderr, "*** Failed to open connection 1 to session bus: %s\n", - error.message); - dbus_error_free (&error); - return 1; - } - - if (!match_acquired_or_lost_signal (conn[1], - "NameAcquired", - dbus_bus_get_unique_name (conn[1]))) - return 1; - - - conn[2] = dbus_bus_get_private (DBUS_BUS_SESSION, &error); - if (dbus_error_is_set (&error)) - { - fprintf (stderr, "*** Failed to open connection 2 to session bus: %s\n", - error.message); - dbus_error_free (&error); - return 1; - } - - if (!match_acquired_or_lost_signal (conn[2], - "NameAcquired", - dbus_bus_get_unique_name (conn[2]))) - return 1; - - - conn[3] = dbus_bus_get_private (DBUS_BUS_SESSION, &error); - if (dbus_error_is_set (&error)) - { - fprintf (stderr, "*** Failed to open connection 3 to session bus: %s\n", - error.message); - dbus_error_free (&error); - return 1; - } - - if (!match_acquired_or_lost_signal (conn[3], - "NameAcquired", - dbus_bus_get_unique_name (conn[3]))) - return 1; - - - monitor = dbus_bus_get (DBUS_BUS_SESSION, &error); - if (dbus_error_is_set (&error)) - { - fprintf (stderr, "*** Failed to open monitoring connection to session bus: %s\n", - error.message); - dbus_error_free (&error); - return 1; - } - - if (!match_acquired_or_lost_signal (monitor, - "NameAcquired", - dbus_bus_get_unique_name (monitor))) - return 1; - - dbus_bus_add_match (monitor, "", &error); - if (dbus_error_is_set (&error)) - { - fprintf (stderr, "*** Failed to set filter on monitoring connection: %s\n", - error.message); - dbus_error_free (&error); - return 1; - } - - - for (i = 0; i < NUM_CONN; i++) - dbus_connection_set_exit_on_disconnect (conn[i], FALSE); - - for (i = 0; i < test_data_len; i++) - { - dbus_uint32_t result; - result = 0; - - if (test_data[i].command == ADD_CONNECTION) - { - result = dbus_bus_request_name (conn[test_data[i].connection_number], - TEST_NAME, - test_data[i].flags, - &error); - - if (dbus_error_is_set (&error)) - { - fprintf (stderr, "Error on addition in iteration %i: %s\n", i, error.message); - dbus_error_free (&error); - return 1; - } - } - else if (test_data[i].command == REMOVE_CONNECTION) - { - result = dbus_bus_release_name (conn[test_data[i].connection_number], - TEST_NAME, - &error); - if (dbus_error_is_set (&error)) - { - fprintf (stderr, "*** Failed to remove connection %i in iteration %i: %s\n", - test_data[i].connection_number, - i, - error.message); - dbus_error_free (&error); - return 1; - } - } - else - { - fprintf (stderr, "Command #%i not a valid command!\n", test_data[i].command); - return 1; - } - - - if (result != test_data[i].expected_result) - { - fprintf (stderr, "Results recived (%i) are not the expected results (%i) in iteration %i\n", - result, - test_data[i].expected_result, - i); - return 1; - } - - if (!check_connection (monitor, i, conn)) - { - fprintf (stderr, "Failed at iteration %i\n", i); - return 1; - } - - if (!check_signals (monitor, i, conn)) - { - fprintf (stderr, "Failed at iteration %i\n", i); - return 1; - } - } - - return 0; -} From b6001f8c30cf6f3f539850865ddb4b6c7773350f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 16 Mar 2010 22:30:50 -0400 Subject: [PATCH 2/9] Release 1.2.22 --- configure.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.in b/configure.in index f583b28e..556b6b22 100644 --- a/configure.in +++ b/configure.in @@ -3,7 +3,7 @@ AC_PREREQ(2.52) m4_define([dbus_major_version], [1]) m4_define([dbus_minor_version], [2]) -m4_define([dbus_micro_version], [20]) +m4_define([dbus_micro_version], [22]) m4_define([dbus_version], [dbus_major_version.dbus_minor_version.dbus_micro_version]) AC_INIT(dbus, [dbus_version]) From bda156f52e377b4e8ad882de120ba94a1d3643b9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 17 Mar 2010 12:43:40 -0400 Subject: [PATCH 3/9] Post-release version bump --- configure.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.in b/configure.in index 556b6b22..074472f2 100644 --- a/configure.in +++ b/configure.in @@ -3,7 +3,7 @@ AC_PREREQ(2.52) m4_define([dbus_major_version], [1]) m4_define([dbus_minor_version], [2]) -m4_define([dbus_micro_version], [22]) +m4_define([dbus_micro_version], [23]) m4_define([dbus_version], [dbus_major_version.dbus_minor_version.dbus_micro_version]) AC_INIT(dbus, [dbus_version]) From aa275ff175180f9635c473bfb56912835aa488af Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Thu, 4 Feb 2010 20:12:28 +0000 Subject: [PATCH 4/9] Move dispatching to destination to bus_dispatch_matches() --- bus/dispatch.c | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/bus/dispatch.c b/bus/dispatch.c index ae6971de..ca55177b 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -86,10 +86,28 @@ bus_dispatch_matches (BusTransaction *transaction, _dbus_assert (sender == NULL || bus_connection_is_active (sender)); _dbus_assert (dbus_message_get_sender (message) != NULL); - connections = bus_transaction_get_connections (transaction); - - dbus_error_init (&tmp_error); context = bus_transaction_get_context (transaction); + + /* First, send the message to the addressed_recipient, if there is one. */ + if (addressed_recipient != NULL) + { + if (!bus_context_check_security_policy (context, transaction, + sender, addressed_recipient, + addressed_recipient, + message, error)) + return FALSE; + + /* Dispatch the message */ + if (!bus_transaction_send (transaction, addressed_recipient, message)) + { + BUS_SET_OOM (error); + return FALSE; + } + } + + /* Now dispatch to others who look interested in this message */ + connections = bus_transaction_get_connections (transaction); + dbus_error_init (&tmp_error); matchmaker = bus_context_get_matchmaker (context); recipients = NULL; @@ -289,24 +307,12 @@ bus_dispatch (DBusConnection *connection, { addressed_recipient = bus_service_get_primary_owners_connection (service); _dbus_assert (addressed_recipient != NULL); - - if (!bus_context_check_security_policy (context, transaction, - connection, addressed_recipient, - addressed_recipient, - message, &error)) - goto out; - - /* Dispatch the message */ - if (!bus_transaction_send (transaction, addressed_recipient, message)) - { - BUS_SET_OOM (&error); - goto out; - } } } - /* Now match the messages against any match rules, which will send - * out signals and such. addressed_recipient may == NULL. + /* Now send the message to its destination (or not, if + * addressed_recipient == NULL), and match it against other connections' + * match rules. */ if (!bus_dispatch_matches (transaction, connection, addressed_recipient, message, &error)) goto out; From af95ba7c8eafaff6c5a8ab8dd2784427b168cab5 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Thu, 4 Feb 2010 20:24:54 +0000 Subject: [PATCH 5/9] Dispatch post-activation messages to anyone interested Previously, if a method call activated a service, it would only be delivered to that service, and not to other services with match rules which should match. This patch replaces the improperly-duplicated dispatch code in activation.c with a call back into the normal dispatch code, fixing this bug (fd.o#26427). --- bus/activation.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/bus/activation.c b/bus/activation.c index 0a28df16..2fcd85d2 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -25,6 +25,7 @@ #include "activation.h" #include "activation-exit-codes.h" #include "desktop-file.h" +#include "dispatch.h" #include "services.h" #include "test.h" #include "utils.h" @@ -1132,21 +1133,12 @@ bus_activation_send_pending_auto_activation_messages (BusActivation *activation addressed_recipient = bus_service_get_primary_owners_connection (service); - /* Check the security policy, which has the side-effect of adding an - * expected pending reply. - */ - if (!bus_context_check_security_policy (activation->context, transaction, - entry->connection, - addressed_recipient, - addressed_recipient, - entry->activation_message, error)) + /* Resume dispatching where we left off in bus_dispatch() */ + if (!bus_dispatch_matches (transaction, + entry->connection, + addressed_recipient, + entry->activation_message, error)) goto error; - - if (!bus_transaction_send (transaction, addressed_recipient, entry->activation_message)) - { - BUS_SET_OOM (error); - goto error; - } } link = next; From c4371e4bc8a561cf91516d22f00c9512dc7e8e53 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 21 Mar 2010 07:01:15 -0400 Subject: [PATCH 6/9] [bus] While creating a syslog, correctly get pointer data from DBusString --- bus/bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bus/bus.c b/bus/bus.c index 3e37e90a..60de72af 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -1196,7 +1196,7 @@ bus_context_log (BusContext *context, DBusSystemLogSeverity severity, const char if (!_dbus_string_append_printf_valist (&full_msg, msg, args)) goto oom_out; - _dbus_system_log (severity, "%s", full_msg); + _dbus_system_log (severity, "%s", _dbus_string_get_const_data (&full_msg)); oom_out: _dbus_string_free (&full_msg); } From 03bb3ce656e9b14fe643572dd0d39f82e955f1f5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 22 Mar 2010 10:38:12 -0400 Subject: [PATCH 7/9] Add DBUS_GNUC_PRINTF checks to new formatting functions Otherwise we don't get GCC warnings. --- bus/bus.c | 3 +++ dbus/dbus-sysdeps.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/bus/bus.c b/bus/bus.c index 60de72af..fd8a8724 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -1175,6 +1175,9 @@ bus_context_get_reply_timeout (BusContext *context) return context->limits.reply_timeout; } +void +bus_context_log (BusContext *context, DBusSystemLogSeverity severity, const char *msg, ...) _DBUS_GNUC_PRINTF (3, 4); + void bus_context_log (BusContext *context, DBusSystemLogSeverity severity, const char *msg, ...) { diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index 12e9124e..7817e04f 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -446,7 +446,7 @@ typedef enum { DBUS_SYSTEM_LOG_FATAL } DBusSystemLogSeverity; -void _dbus_system_log (DBusSystemLogSeverity severity, const char *msg, ...); +void _dbus_system_log (DBusSystemLogSeverity severity, const char *msg, ...) _DBUS_GNUC_PRINTF (2, 3); void _dbus_system_logv (DBusSystemLogSeverity severity, const char *msg, va_list args); /* Define DBUS_VA_COPY() to do the right thing for copying va_list variables. From c93d3ec2ff13f31291c56f6d5d4f7a77ecdb5ea7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 22 Mar 2010 11:50:24 -0400 Subject: [PATCH 8/9] Add DBUS_INT64_MODIFIER define, turn on -Wformat https://bugs.freedesktop.org/show_bug.cgi?id=19195 We were previously using -Wno-format because we didn't have a #define for DBUS_INT64_MODIFIER, which was really lame because it easily hid problems. For now, just define it if we're on glibc; this is obviously not strictly correct but it's safe, because the formatting is only used in DBUS_VERBOSE mode, and in tools/dbus-monitor. Ideally we get the the glib code relicensed. --- configure.in | 45 ++++++++++++++++++++++++++---- dbus/dbus-connection.c | 2 +- dbus/dbus-credentials.c | 4 +-- dbus/dbus-marshal-basic.c | 10 ++----- dbus/dbus-marshal-recursive-util.c | 4 +-- tools/dbus-print-message.c | 9 ++++-- 6 files changed, 53 insertions(+), 21 deletions(-) diff --git a/configure.in b/configure.in index 074472f2..3fae8353 100644 --- a/configure.in +++ b/configure.in @@ -146,6 +146,31 @@ if test x$enable_gcov = xyes; then fi AM_CONDITIONAL(DBUS_GCOV_ENABLED, test x$enable_gcov = xyes) +# glibc21.m4 serial 3 +dnl Copyright (C) 2000-2002, 2004 Free Software Foundation, Inc. +dnl This file is free software; the Free Software Foundation +dnl gives unlimited permission to copy and/or distribute it, +dnl with or without modifications, as long as this notice is preserved. + +# Test for the GNU C Library, version 2.1 or newer. +# From Bruno Haible. + +AC_CACHE_CHECK(whether we are using the GNU C Library 2.1 or newer, + ac_cv_gnu_library_2_1, + [AC_EGREP_CPP([Lucky GNU user], + [ +#include +#ifdef __GNU_LIBRARY__ + #if (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 1) || (__GLIBC__ > 2) + Lucky GNU user + #endif +#endif + ], + ac_cv_gnu_library_2_1=yes, + ac_cv_gnu_library_2_1=no) + ] +) + #### Integer sizes AC_CHECK_SIZEOF(char) @@ -164,21 +189,32 @@ $ac_cv_sizeof_int) dbusint64=int dbusint64_constant='(val)' dbusuint64_constant='(val)' + dbusint64_printf_modifier='""' ;; $ac_cv_sizeof_long) dbusint64=long dbusint64_constant='(val##L)' dbusuint64_constant='(val##UL)' + dbusint64_printf_modifier='"l"' ;; $ac_cv_sizeof_long_long) dbusint64='long long' dbusint64_constant='(val##LL)' dbusuint64_constant='(val##ULL)' + # Ideally we discover what the format is, but this is + # only used in verbose mode, so eh... + if test x"$ac_cv_gnu_library_2_1" = xyes; then + dbusint64_printf_modifier='"ll"' + fi ;; $ac_cv_sizeof___int64) dbusint64=__int64 dbusint64_constant='(val##i64)' dbusuint64_constant='(val##ui64)' + # See above case + if test x"$ac_cv_gnu_library_2_1" = xyes; then + dbusint64_printf_modifier='"ll"' + fi ;; esac @@ -193,6 +229,9 @@ else DBUS_HAVE_INT64=1 DBUS_INT64_CONSTANT="$dbusint64_constant" DBUS_UINT64_CONSTANT="$dbusuint64_constant" + if test x"$dbusint64_printf_modifier" != x; then + AC_DEFINE_UNQUOTED(DBUS_INT64_PRINTF_MODIFIER, [$dbusint64_printf_modifier], [Define to printf modifier for 64 bit integer type]) + fi AC_MSG_RESULT($DBUS_INT64_TYPE) fi @@ -1089,12 +1128,6 @@ if test "x$GCC" = "xyes"; then CFLAGS="$CFLAGS -Wno-pointer-sign" fi ;; - esac - - # http://bugs.freedesktop.org/show_bug.cgi?id=19195 - case " $CFLAGS " in - *[\ \ ]-Wno-format[\ \ ]*) ;; - *) CFLAGS="$CFLAGS -Wno-format" ;; esac # This one is special - it's not a warning override. diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 69fdf530..9526d3cc 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -2434,7 +2434,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) goto recheck_status; } - _dbus_verbose ("dbus_connection_send_with_reply_and_block(): Waited %ld milliseconds and got no reply\n", + _dbus_verbose ("dbus_connection_send_with_reply_and_block(): Waited %d milliseconds and got no reply\n", elapsed_milliseconds); _dbus_assert (!_dbus_pending_call_get_completed_unlocked (pending)); diff --git a/dbus/dbus-credentials.c b/dbus/dbus-credentials.c index 889166ba..50a0548a 100644 --- a/dbus/dbus-credentials.c +++ b/dbus/dbus-credentials.c @@ -519,13 +519,13 @@ _dbus_credentials_to_string_append (DBusCredentials *credentials, join = FALSE; if (credentials->unix_uid != DBUS_UID_UNSET) { - if (!_dbus_string_append_printf (string, "uid=%d", credentials->unix_uid)) + if (!_dbus_string_append_printf (string, "uid=%d", (int) credentials->unix_uid)) goto oom; join = TRUE; } if (credentials->unix_pid != DBUS_PID_UNSET) { - if (!_dbus_string_append_printf (string, "%spid=%d", join ? " " : "", credentials->unix_pid)) + if (!_dbus_string_append_printf (string, "%spid=%d", join ? " " : "", (int) credentials->unix_pid)) goto oom; join = TRUE; } diff --git a/dbus/dbus-marshal-basic.c b/dbus/dbus-marshal-basic.c index 52308cbe..3d0d3d0b 100644 --- a/dbus/dbus-marshal-basic.c +++ b/dbus/dbus-marshal-basic.c @@ -1386,15 +1386,9 @@ _dbus_verbose_bytes (const unsigned char *data, if (i > 7 && _DBUS_ALIGN_ADDRESS (&data[i], 8) == &data[i]) { -#ifdef DBUS_HAVE_INT64 - /* I think I probably mean "GNU libc printf" and not "GNUC" - * but we'll wait until someone complains. If you hit this, - * just turn off verbose mode as a workaround. - */ -#if __GNUC__ - _dbus_verbose (" u64: 0x%llx", +#ifdef DBUS_INT64_PRINTF_MODIFIER + _dbus_verbose (" u64: 0x%" DBUS_INT64_PRINTF_MODIFIER "x", *(dbus_uint64_t*)&data[i-8]); -#endif #endif _dbus_verbose (" dbl: %g", *(double*)&data[i-8]); diff --git a/dbus/dbus-marshal-recursive-util.c b/dbus/dbus-marshal-recursive-util.c index f87644c7..f4d59f3f 100644 --- a/dbus/dbus-marshal-recursive-util.c +++ b/dbus/dbus-marshal-recursive-util.c @@ -2651,8 +2651,8 @@ double_read_value (TestTypeNode *node, if (!_DBUS_DOUBLES_BITWISE_EQUAL (v, expected)) { -#ifdef DBUS_HAVE_INT64 - _dbus_warn ("Expected double %g got %g\n bits = 0x%llx vs.\n bits = 0x%llx)\n", +#ifdef DBUS_INT64_PRINTF_MODIFIER + _dbus_warn ("Expected double %g got %g\n bits = 0x%" DBUS_INT64_PRINTF_MODIFIER "x vs.\n bits = 0x%" DBUS_INT64_PRINTF_MODIFIER "x)\n", expected, v, *(dbus_uint64_t*)(char*)&expected, *(dbus_uint64_t*)(char*)&v); diff --git a/tools/dbus-print-message.c b/tools/dbus-print-message.c index 8a8e351d..cac40410 100644 --- a/tools/dbus-print-message.c +++ b/tools/dbus-print-message.c @@ -22,6 +22,7 @@ #include "dbus-print-message.h" #include +#include "config.h" static const char* type_to_name (int message_type) @@ -225,7 +226,9 @@ print_iter (DBusMessageIter *iter, dbus_bool_t literal, int depth) { dbus_int64_t val; dbus_message_iter_get_basic (iter, &val); - printf ("int64 %lld\n", val); +#ifdef DBUS_INT64_PRINTF_MODIFIER + printf ("int64 %" DBUS_INT64_PRINTF_MODIFIER "d\n", val); +#endif break; } @@ -233,7 +236,9 @@ print_iter (DBusMessageIter *iter, dbus_bool_t literal, int depth) { dbus_uint64_t val; dbus_message_iter_get_basic (iter, &val); - printf ("uint64 %llu\n", val); +#ifdef DBUS_INT64_PRINTF_MODIFIER + printf ("uint64 %" DBUS_INT64_PRINTF_MODIFIER "u\n", val); +#endif break; } From 3861cb42f437a9eb2b13c5c2aa4081268c45c32c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 22 Mar 2010 14:33:37 -0400 Subject: [PATCH 9/9] [64 bit printf] Update to use DBUS_PID_FORMAT, print (omitted) There were already defines for formatting pids and uids, so use those. In the case where we don't have a format specifier for 64 bit, print (omitted) in dbus-monitor. --- dbus/dbus-credentials.c | 4 ++-- tools/dbus-print-message.c | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/dbus/dbus-credentials.c b/dbus/dbus-credentials.c index 50a0548a..ff69f3b0 100644 --- a/dbus/dbus-credentials.c +++ b/dbus/dbus-credentials.c @@ -519,13 +519,13 @@ _dbus_credentials_to_string_append (DBusCredentials *credentials, join = FALSE; if (credentials->unix_uid != DBUS_UID_UNSET) { - if (!_dbus_string_append_printf (string, "uid=%d", (int) credentials->unix_uid)) + if (!_dbus_string_append_printf (string, "uid=" DBUS_UID_FORMAT, credentials->unix_uid)) goto oom; join = TRUE; } if (credentials->unix_pid != DBUS_PID_UNSET) { - if (!_dbus_string_append_printf (string, "%spid=%d", join ? " " : "", (int) credentials->unix_pid)) + if (!_dbus_string_append_printf (string, "%spid=" DBUS_PID_FORMAT, join ? " " : "", credentials->unix_pid)) goto oom; join = TRUE; } diff --git a/tools/dbus-print-message.c b/tools/dbus-print-message.c index cac40410..fac5cc1f 100644 --- a/tools/dbus-print-message.c +++ b/tools/dbus-print-message.c @@ -227,7 +227,9 @@ print_iter (DBusMessageIter *iter, dbus_bool_t literal, int depth) dbus_int64_t val; dbus_message_iter_get_basic (iter, &val); #ifdef DBUS_INT64_PRINTF_MODIFIER - printf ("int64 %" DBUS_INT64_PRINTF_MODIFIER "d\n", val); + printf ("int64 %" DBUS_INT64_PRINTF_MODIFIER "d\n", val); +#else + printf ("int64 (omitted)\n"); #endif break; } @@ -237,7 +239,9 @@ print_iter (DBusMessageIter *iter, dbus_bool_t literal, int depth) dbus_uint64_t val; dbus_message_iter_get_basic (iter, &val); #ifdef DBUS_INT64_PRINTF_MODIFIER - printf ("uint64 %" DBUS_INT64_PRINTF_MODIFIER "u\n", val); + printf ("uint64 %" DBUS_INT64_PRINTF_MODIFIER "u\n", val); +#else + printf ("uint64 (omitted)\n"); #endif break; }