[Bug 28015] TpChannelDispatchOperation: convenience API to Claim and reject CDOs

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=28015

--- Comment #6 from Will Thompson <will.thompson at collabora.co.uk> 2011-05-25 08:59:35 PDT ---
Review of attachment 47117:
 --> (https://bugs.freedesktop.org/review?bug=28015&attachment=47117)

Looks functionally fine but I have some refcounting and other nitpicks.

::: telepathy-glib/channel-dispatch-operation.c
@@ +1446,3 @@
   if (!tp_channel_dispatch_operation_claim_finish (self, result, &error))
     {
+      DEBUG ("Failed to Calim %s: %s",

Calim is my best friend's name.

@@ +1452,3 @@
+      g_simple_async_result_complete (ctx->result);
+      /* Remove the ref we got from the caller */
+      g_object_unref (ctx->result);

This is not very clear to me, really: you're having to do “more” freeing of the
ctx on some but not all code paths, just so that the callback has to do its own
unreffing of the result object?

Also this failure path is duplicated. I don't mind that much though. I do mind
'goto out' when there's exactly one line of code between the goto and the label
and you could easily add an else block.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.