[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 #9 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2011-05-26 03:27:07 PDT ---
(In reply to comment #6)
> Review of attachment 47117 [details]:
> 
> 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.

fixed (squashed).

> @@ +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?

As said in prepare_core_and_claim's documentation, it takes the ref on @result
and pass it back to @callback is everything is fine. But if an error occurs,
@callback won't be called, the operation will be completed (with an error) and
so we have to drop this ref.

I made that clearer in the code.

> 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.

I factored out prepare_core_and_claim_ctx_failed() removing the code
duplication.

-- 
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.