GDBusConnection: move 'Set' typecheck to worker
authorRyan Lortie <desrt@desrt.ca>
Sat, 22 Jun 2013 17:37:54 +0000 (13:37 -0400)
committerRyan Lortie <desrt@desrt.ca>
Sat, 22 Jun 2013 17:37:54 +0000 (13:37 -0400)
We presently do a lot of checks on property sets (signature check,
correct interface, property exists, etc.) from the worker thread before
dispatching the call to the user's thread.  The typecheck, however, is
saved until just before calling the user's vfunc, in their thread.

My best guess is that this was done to save having to unpack the value
from the tuple twice (since we don't unpack it until we're just about
the call the user).

This patch moves the check to the same place as all of the other checks.

The purpose of this change is to allow for sharing this check with the
(soon-to-be-introduced) case of handing property sets from
method_call().

This change has a minor side effect: error messages generated by sending
invalid values to property sets are no longer guaranteed to be correctly
ordered with respect to the void returns from successful property sets.
They will instead be correctly ordered with respect to the other error
messages.

https://bugzilla.gnome.org/show_bug.cgi?id=698375

gio/gdbusconnection.c

index 231e0d2..d5898f7 100644 (file)
@@ -4195,20 +4195,6 @@ invoke_set_property_in_idle_cb (gpointer _data)
                  NULL,
                  &value);
 
-  /* Fail with org.freedesktop.DBus.Error.InvalidArgs if the type
-   * of the given value is wrong
-   */
-  if (g_strcmp0 (g_variant_get_type_string (value), data->property_info->signature) != 0)
-    {
-      reply = g_dbus_message_new_method_error (data->message,
-                                               "org.freedesktop.DBus.Error.InvalidArgs",
-                                               _("Error setting property '%s': Expected type '%s' but got '%s'"),
-                                               data->property_info->name,
-                                               data->property_info->signature,
-                                               g_variant_get_type_string (value));
-      goto out;
-    }
-
   if (!data->vtable->set_property (data->connection,
                                    g_dbus_message_get_sender (data->message),
                                    g_dbus_message_get_path (data->message),
@@ -4232,7 +4218,6 @@ invoke_set_property_in_idle_cb (gpointer _data)
       reply = g_dbus_message_new_method_reply (data->message);
     }
 
- out:
   g_assert (reply != NULL);
   g_dbus_connection_send_message (data->connection, reply, G_DBUS_SEND_MESSAGE_FLAGS_NONE, NULL, NULL);
   g_object_unref (reply);
@@ -4328,6 +4313,31 @@ validate_and_maybe_schedule_property_getset (GDBusConnection            *connect
       goto out;
     }
 
+  if (!is_get)
+    {
+      GVariant *value;
+
+      /* Fail with org.freedesktop.DBus.Error.InvalidArgs if the type
+       * of the given value is wrong
+       */
+      g_variant_get_child (g_dbus_message_get_body (message), 2, "v", &value);
+      if (g_strcmp0 (g_variant_get_type_string (value), property_info->signature) != 0)
+        {
+          reply = g_dbus_message_new_method_error (message,
+                                                   "org.freedesktop.DBus.Error.InvalidArgs",
+                                                   _("Error setting property '%s': Expected type '%s' but got '%s'"),
+                                                   property_name, property_info->signature,
+                                                   g_variant_get_type_string (value));
+          g_dbus_connection_send_message_unlocked (connection, reply, G_DBUS_SEND_MESSAGE_FLAGS_NONE, NULL, NULL);
+          g_variant_unref (value);
+          g_object_unref (reply);
+          handled = TRUE;
+          goto out;
+        }
+
+      g_variant_unref (value);
+    }
+
   /* ok, got the property info - call user code in an idle handler */
   property_data = g_new0 (PropertyData, 1);
   property_data->connection = g_object_ref (connection);