From 31296eab8363f8b865ad83e00b848f27c683907b Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 9 Sep 2014 15:03:25 +0100 Subject: [PATCH 1/3] fdpass test: respond to the simpler bits of Alban's review - rename server_message_cb to left_server_message_cb - open /dev/null instead of /dev/zero, and just crash if we can't - when the left connection should be disconnected, assert that both ends disconnect; when it should not, assert that - always assert that the right connection is not disconnected - make my_test_skip() inline since it's trivial, it will not currently be used on fd-passing platforms, and inline functions are immune to -Wunused --- test/fdpass.c | 81 +++++++++++++++++++++++++++++++---------------------------- 1 file changed, 43 insertions(+), 38 deletions(-) diff --git a/test/fdpass.c b/test/fdpass.c index cdbdcaf..282de60 100644 --- a/test/fdpass.c +++ b/test/fdpass.c @@ -80,7 +80,7 @@ typedef struct { #if !GLIB_CHECK_VERSION (2, 38, 0) #define g_test_skip(s) my_test_skip (s) -static void my_test_skip (const gchar *s) +static inline void my_test_skip (const gchar *s) { g_message ("SKIP: %s", s); } @@ -102,7 +102,7 @@ assert_no_error (const DBusError *e) } static DBusHandlerResult -server_message_cb (DBusConnection *server_conn, +left_server_message_cb (DBusConnection *server_conn, DBusMessage *message, void *data) { @@ -146,7 +146,7 @@ new_conn_cb (DBusServer *server, f->left_server_conn = dbus_connection_ref (server_conn); if (!dbus_connection_add_filter (server_conn, - server_message_cb, f, NULL)) + left_server_message_cb, f, NULL)) oom ("adding filter"); } else @@ -252,10 +252,13 @@ setup (Fixture *f, new_conn_cb, f, NULL); test_server_setup (f->ctx, f->server); - f->fd_before = open ("/dev/zero", O_RDONLY); + f->fd_before = open ("/dev/null", O_RDONLY); - if (f->fd_before >= 0) - _dbus_fd_set_close_on_exec (f->fd_before); + /* this should succeed on any reasonable Unix */ + if (f->fd_before < 0) + g_error ("cannot open /dev/null for reading: %s", g_strerror (errno)); + + _dbus_fd_set_close_on_exec (f->fd_before); #endif } @@ -272,12 +275,6 @@ test_relay (Fixture *f, struct stat stat_before; struct stat stat_after; - if (f->fd_before < 0) - { - g_test_skip ("cannot open /dev/zero"); - return; - } - test_connect (f, data); outgoing = dbus_message_new_signal ("/com/example/Hello", @@ -339,6 +336,11 @@ test_relay (Fixture *f, if (close (fd_after) < 0) g_error ("%s", g_strerror (errno)); + + g_assert (dbus_connection_get_is_connected (f->right_client_conn)); + g_assert (dbus_connection_get_is_connected (f->right_server_conn)); + g_assert (dbus_connection_get_is_connected (f->left_client_conn)); + g_assert (dbus_connection_get_is_connected (f->left_server_conn)); #else g_test_skip ("fd-passing not supported on this platform"); #endif @@ -353,12 +355,6 @@ test_limit (Fixture *f, DBusMessage *outgoing, *incoming; int i; - if (f->fd_before < 0) - { - g_test_skip ("cannot open /dev/zero"); - return; - } - test_connect (f, data); outgoing = dbus_message_new_signal ("/com/example/Hello", @@ -399,6 +395,11 @@ test_limit (Fixture *f, g_assert_cmpuint (dbus_message_get_serial (incoming), ==, serial); dbus_message_unref (incoming); + + g_assert (dbus_connection_get_is_connected (f->right_client_conn)); + g_assert (dbus_connection_get_is_connected (f->right_server_conn)); + g_assert (dbus_connection_get_is_connected (f->left_client_conn)); + g_assert (dbus_connection_get_is_connected (f->left_server_conn)); #else g_test_skip ("fd-passing not supported on this platform"); #endif @@ -412,12 +413,6 @@ test_too_many (Fixture *f, DBusMessage *outgoing; int i; - if (f->fd_before < 0) - { - g_test_skip ("cannot open /dev/zero"); - return; - } - test_connect (f, data); outgoing = dbus_message_new_signal ("/com/example/Hello", @@ -438,7 +433,8 @@ test_too_many (Fixture *f, dbus_message_unref (outgoing); /* The sender is unceremoniously disconnected. */ - while (dbus_connection_get_is_connected (f->left_client_conn)) + while (dbus_connection_get_is_connected (f->left_client_conn) || + dbus_connection_get_is_connected (f->left_server_conn)) { g_print ("."); test_main_context_iterate (f->ctx, TRUE); @@ -446,6 +442,11 @@ test_too_many (Fixture *f, /* The message didn't get through without its fds. */ g_assert_cmpuint (g_queue_get_length (&f->messages), ==, 0); + + /* The intended victim is unaffected by the left connection's + * misbehaviour. */ + g_assert (dbus_connection_get_is_connected (f->right_client_conn)); + g_assert (dbus_connection_get_is_connected (f->right_server_conn)); #else g_test_skip ("fd-passing not supported on this platform"); #endif @@ -460,12 +461,6 @@ test_flood (Fixture *f, DBusMessage *outgoing[SOME_MESSAGES]; dbus_uint32_t serial; - if (f->fd_before < 0) - { - g_test_skip ("cannot open /dev/zero"); - return; - } - test_connect (f, data); for (j = 0; j < SOME_MESSAGES; j++) @@ -520,6 +515,11 @@ test_flood (Fixture *f, dbus_message_unref (incoming); } + + g_assert (dbus_connection_get_is_connected (f->right_client_conn)); + g_assert (dbus_connection_get_is_connected (f->right_server_conn)); + g_assert (dbus_connection_get_is_connected (f->left_client_conn)); + g_assert (dbus_connection_get_is_connected (f->left_server_conn)); #else g_test_skip ("fd-passing not supported on this platform"); #endif @@ -533,12 +533,6 @@ test_odd_limit (Fixture *f, DBusMessage *outgoing; int i; - if (f->fd_before < 0) - { - g_test_skip ("cannot open /dev/zero"); - return; - } - test_connect (f, data); dbus_connection_set_max_message_unix_fds (f->left_server_conn, 7); dbus_connection_set_max_message_unix_fds (f->right_server_conn, 7); @@ -563,7 +557,8 @@ test_odd_limit (Fixture *f, if (GPOINTER_TO_INT (data) > 0) { /* The sender is unceremoniously disconnected. */ - while (dbus_connection_get_is_connected (f->left_client_conn)) + while (dbus_connection_get_is_connected (f->left_client_conn) || + dbus_connection_get_is_connected (f->left_server_conn)) { g_print ("."); test_main_context_iterate (f->ctx, TRUE); @@ -571,6 +566,11 @@ test_odd_limit (Fixture *f, /* The message didn't get through without its fds. */ g_assert_cmpuint (g_queue_get_length (&f->messages), ==, 0); + + /* The intended victim is unaffected by the left connection's + * misbehaviour. */ + g_assert (dbus_connection_get_is_connected (f->right_client_conn)); + g_assert (dbus_connection_get_is_connected (f->right_server_conn)); } else { @@ -598,6 +598,11 @@ test_odd_limit (Fixture *f, "/com/example/Hello"); dbus_message_unref (incoming); + + g_assert (dbus_connection_get_is_connected (f->right_client_conn)); + g_assert (dbus_connection_get_is_connected (f->right_server_conn)); + g_assert (dbus_connection_get_is_connected (f->left_client_conn)); + g_assert (dbus_connection_get_is_connected (f->left_server_conn)); } #else g_test_skip ("fd-passing not supported on this platform"); -- 2.1.0