New plugin API

[Ankit Vani]

Hi Richard

On 9 August 2013 11:24, Richard Laager <rlaager at wiktel.com> wrote:
> On Fri, 2013-08-09 at 01:07 +0530, Ankit Vani wrote:
>> 3. Memory management becomes a problem with needing to remember to unref
>> returned GPlugin objects and lists all the time, since the rest of the
>> GObjectified libpurple does not ref and unref unless necessary.
>
> Can you discuss this a little more?
>
> I find the following code highly suspect, though Gary said it was fine
> (but I don't understand how):
>
> 199     info = gplugin_plugin_get_info(plugin);
> 200     g_object_unref(info);
> 201
> 202     return PURPLE_PLUGIN_INFO(info);
>
> and
>
> 830     plugin = gplugin_plugin_manager_find_plugin(id);
> 831     g_object_unref(plugin);
> 832
> 833     return plugin;
>
> What is stopping g_object_unref() from immediately freeing info and
> plugin, respectively?

So far in the gobjectification of purple, we use objects in a way similar to
this:
if (purple_account_is_connected(purple_something_get_account()))
    /* do something */

or:
PurpleAccount *account = purple_something_get_account();
/* do something with account */

However, if there is some place where an account could continue to be used even
after said account is destroyed, and we want to avoid destruction in such a
case, we may do g_object_ref(account) while assigning it there, and
g_object_unref when we're done with it.

However, GPlugin refs everything before returning it to us. So basically you
would need to do things like:

GPluginPluginInfo *info = gplugin_plugin_get_info(plugin);

if (!gplugin_plugin_info_get_name()) {
    g_object_unref(info);
    return FALSE;
}

g_object_unref(info);
return TRUE;

This is not really a problem but it is different from how other GObjectified
entities in libpurple are used. And someone might at some point forget to unref
a retrieved object, causing a leak.

And as with other purple objects, we can ref and unref them only where necessary
for the sake of uniformity and simplicity.

>> A plugin is also not loadable if it does not return an info instance. However,
>> purple_plugin_get_error() will return NULL in this case because there would be
>> nowhere to store the error.
>
> purple_plugin_get_error() treats purple_plugin_get_info() returning NULL
> as an assertion failure--you call g_return_val_if_fail(). If that's a
> scenario that can occur for reasons outside of libpurple's control, you
> should use an explicit check. Then, you can just return a (translated)
> fixed string describing the error.
>
> And purple_plugin_get_error() probably should return a const gchar *,
> right? I assume the caller isn't supposed to modify that value.

Yes, you're right. I'll make the changes. :)

>> Protocol plugins are to be both internal and load-on-query.
>
> I assume you mean *in-tree* plugins which provide protocols. Did you
> decide against just always compiling those statically, and if so, why?

Yes, I meant in-tree plugins that provide protocols.

I haven't come to a decision about whether we should always compile in-tree
protocols statically. There are merits (e.g. lets distributions ship certain
protocols with additional dependencies seperately) and demerits on both sides
and I don't suppose this is a decision I can come to by myself. I would like to
know what the community thinks of always statically linking protocols.

For now, I have retained the current behaviour and the in-tree protocol plugins
behave the way they used to, and can be linked both dynamically and statically
like they used to.

                                                                         - Ankit