[Bug 44331] Gabble plugin API symbols should be factored out to a separate library

[bugzilla-daemon at freedesktop.org]

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

--- Comment #20 from Olli Salli <ollisal at gmail.com> 2012-01-09 06:19:35 PST ---
(In reply to comment #18)
> Why are you returning the GAsyncResult in the create_sidecar_async function?
> That's really weird.

Ah, you're absolutely right here; this is not GiOdiomatic at all. My bad! I
guess this misconception of mine came from the Qt APIs, where we return pending
operation / job objects, and you can use the pointer to them to associate a
particular invocation with some context. But in GIO style, you use the
user_data for passing context.

So Siraj, the return type of the _async functions can be changed to void.
That's 
fairly simple as we don't even use the return value anywhere.

> 
>    g_object_unref (result);
> +  return g_object_ref (sidecar);
> 
> What is this I don't even? Store the GAsyncResult in the private struct of the
> plugin, complete it (in an idle if necessary to make sure it re-enters the
> mainloop) and unref & clear it straight afterwards. Returning a new ref to the
> sidecar is correct, though.

We don't even have to store it at all for the plugins I've looked at, because
they all actually complete the operation already in the _async function
starting it, through the mainloop as you say.

So Siraj: for this plugin and ytstenut, you can just keep the g_object_unref in
the operation start function, after the call to
g_simple_async_result_complete_in_idle() as it were. That's fairly natural once
the return type is void... because then the only pointer in scope is held by
GLib internals, so we must've dropped all of our object references at that
point as well.

for any plugins which actually do some further asynchronous steps before
completing the op: please store the op as Jonny says. But in any case, unref it
immediately after the call to the g_simple_async_result_complete(_in_idle)
function and setting it back to NULL if stored.

Additional observation:

+  iface->create_sidecar_async = test_plugin_create_sidecar;

would be more consistent to rename the implementation func to have _async
suffix as well?

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