Created attachment 239211 [details] [review]
g_dbus_connection_signal_subscribe: add path and namespace matching

Review of attachment 239211 [details] [review]:

I'm happy with the API.  Implementation needs to have some issues addressed.

::: gio/gdbusconnection.c
@@ +3246,3 @@
   gchar *member;
   gchar *object_path;
   gchar *arg0;

could also add the flags here instead, since it's only possible for one of these three to ever be set...

@@ +3477,3 @@
+    {
+      arg0namespace = arg0;
+      arg0 = NULL;

arg0 = NULL here is a bit gross -- another argument for just storing the flags inside of the rule.

@@ +3527,3 @@
   signal_data->arg0                  = g_strdup (arg0);
+  signal_data->arg0namespace         = g_strdup (arg0namespace);
+  signal_data->arg0path              = g_strdup (arg0path);

I don't see any g_free() added for these....

@@ +3767,3 @@
+                   const gchar *arg0)
+{
+  return g_strcmp0 (rule, arg0) == 0 ||

you could do a lot better to optimise this code a bit...

also: g_str_equal() is generally preferred to g_strcmp0() for cases like this.


fwiw, dconf does this:

  int len_a, len_b;

  len_a = strlen (path_a);
  len_b = strlen (path_b);

  if (len_a < len_b && path_a[len_a - 1] != '/')
    return FALSE;

  if (len_b < len_a && path_b[len_b -1] != '/')
    return FALSE;

  return memcmp (path_a, path_b, MIN (len_a, len_b)) == 0;

@@ +3827,3 @@
       if (signal_data->arg0 != NULL && g_strcmp0 (signal_data->arg0, arg0) != 0)
         continue;
+      else if (signal_data->arg0namespace != NULL && !g_str_has_prefix (arg0, signal_data->arg0namespace))

having a prefix of "org.Foo" will match "org.FooBar.OtherThing"

::: gio/tests/gdbus-connection.c
@@ +774,3 @@
+
+  test_match_rule (con, G_DBUS_SIGNAL_FLAGS_MATCH_ARG0_NAMESPACE, "org.gtk", "org.gtk.Example", FALSE);
+  test_match_rule (con, G_DBUS_SIGNAL_FLAGS_MATCH_ARG0_NAMESPACE, "org.gtk", "org.gtk.Example", FALSE);

should add a test for the org.Foo vs. org.FooBar.OtherThing situation...

Created attachment 240414 [details] [review]
g_dbus_connection_signal_subscribe: add path and namespace matching

Review of attachment 240414 [details] [review]:

Very much improved this time around.  Go ahead.

::: gio/gdbusconnection.c
@@ +3471,3 @@
   g_return_val_if_fail (callback != NULL, 0);
   g_return_val_if_fail (check_initialized (connection), 0);
+  g_return_val_if_fail (!((flags & G_DBUS_SIGNAL_FLAGS_MATCH_ARG0_PATH) && (flags & G_DBUS_SIGNAL_FLAGS_MATCH_ARG0_NAMESPACE)), 0);

a bit odd that we'd accept these flags even if arg0 is NULL... no strong opinion, though.

Created attachment 240926 [details] [review]
g_dbus_connection_signal_subscribe: add path and namespace matching

Agreed.