Attachment 105989 Details for Bug 83622 – fdpass test: respond to the simpler bits of Alban's review

[patch] fdpass test: respond to the simpler bits of Alban's review

0001-fdpass-test-respond-to-the-simpler-bits-of-Alban-s-r.patch (text/plain), 7.29 KB, created by Simon McVittie on 2014-09-09 15:16:11 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2014-09-09 15:16:11 UTC

Size: 7.29 KB

patch

obsolete

>From 31296eab8363f8b865ad83e00b848f27c683907b Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>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
>

Actions: View | Splinter Review

Attachments on bug 83622: 105917 | 105918 | 105975 | 105976 | 105989 | 105990 | 105991 | 105996 | 105997 | 106177 | 106178 | 106187 | 106194

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.