Created attachment 38288 [details] [review]
Watch server_set_data failure in dbus__server_setup_with_g_main

Created attachment 38289 [details] [review]
Assert that marshalling in dbus_g_method_return succeeded

Created attachment 38290 [details] [review]
Verify call in dbus glib test

Review of attachment 38289 [details] [review]:

::: dbus/dbus-gobject.c
@@ +2713,3 @@
 	}
+      marshalled = _dbus_gvalue_marshal (&iter, &value);
+      g_assert (marshalled);

Maybe the call to _dbus_gvalue_marshal() should be in an else block; and we should have a g_critical() with some context rather than an assertion? c.f. emit_signal_for_registration()

Review of attachment 38289 [details] [review]:

::: dbus/dbus-gobject.c
@@ +2713,3 @@
 	}
+      marshalled = _dbus_gvalue_marshal (&iter, &value);
+      g_assert (marshalled);

Maybe the call to _dbus_gvalue_marshal() should be in an else block; and we should have a g_critical() with some context rather than an assertion? c.f. emit_signal_for_registration()

The other two patches look good.

Created attachment 41068 [details] [review]
Show warning if marshalling in dbus_g_method_return fails

Updated patch for dbus_g_method_return, so it warns instead of asserting.

Created attachment 41069 [details] [review]
Verify Increment call in dbus-glib-test

This patch actually uses 'lose' as used elsewhere in the test, instead of assert.

Review of attachment 41069 [details] [review]:

Looks fine, r+ from me.

Review of attachment 38288 [details] [review]:

Looks fine. r+ from wjt already, r+ from me too.

Review of attachment 41068 [details] [review]:

::: dbus/dbus-gobject.c
@@ +2950,3 @@
+          if (!_dbus_gvalue_marshal (&iter, &value))
+            g_warning ("failed to marshal parameter %d for method %s",
+                       i, dbus_message_get_member (reply));

Looks good in principle, but replies don't have a member name. I think you want dbus_message_get_member (context->message), or just context->method.

Created attachment 41622 [details] [review]
Get member name from the method call message, not the reply, for warnings

Attachment 41069 [details] and Attachment 38288 [details] pushed to master. I attach a patch to fix the warning for Attachment 41068 [details], to be applied after that one.

Fixed in git for 0.96, based on IRC review from Cosimo Alfarano and no veto from reviewers.