[Aleksander Morgado]

Hey!

On Tue, Dec 22, 2015 at 5:27 PM, Carlo Lobrano <c.lobrano at gmail.com> wrote:
> I am using cinterion and other plugins as reference to implement
> load_supported_bands for Telit plugin, but there is a point which is not
> clear for me.
>
> In my reference plugins I've always seen this kind of result assignement
>
> supported_bands_ready (...) {
>        /* get the results */
>        g_simple_async_result_set_op_res_gpointer (simple,
>                                                    bands,
>                                                    (GDestroyNotify)
> g_array_unref);
>        g_simple_async_result_complete (operation_result);
>        g_object_unref (operation_result);
> }
>
> where bands is a GArray* that might contain the list of supported bands if
> everything went fine or NULL otherwise.
>

Yes, in the specific case of the cinterion plugin a newly allocated
GArray* is returned by mm_cinterion_parse_scfg_test(). What we're
doing here is to put that newly allocated GArray in the result, and
say that whenever the result is disposed, the array should be disposed
as well (i.e. the array is now owned by the result object). There is
no memory leak here, because once the result is unref-ed for the last
time, it will also dispose the GArray.

> My question is: there is any kind of memory leak here? Is it correct to use
> g_array_ref(bands) instead?
>
> supported_bands_ready (...) {
>        /* get the results */
>        g_simple_async_result_set_op_res_gpointer (simple,
>                                                    g_array_ref(bands),
>                                                    (GDestroyNotify)
> g_array_unref);
>        g_simple_async_result_complete (operation_result);
>        g_object_unref (operation_result);
> }

It would be ok to pass a newly created reference with g_array_ref(),
but only if you afterwards dispose the one you got from
mm_cinterion_parse_scfg_test(), or you would be leaking a reference.
In other words, these two blocks would be both ok:

1) ========
if (mm_cinterion_parse_scfg_test (&bands)) {
    g_simple_async_result_set_op_res_gpointer (simple, bands,
(GDestroyNotify) g_array_unref ());
    g_simple_async_result_complete (simple);
    g_object_unref (simple);
}

2) ========
if (mm_cinterion_parse_scfg_test (&bands)) {
    g_simple_async_result_set_op_res_gpointer (simple, g_array_ref
(bands), (GDestroyNotify) g_array_unref ());
    g_array_unref  (bands);
    g_simple_async_result_complete (simple);
    g_object_unref (simple);
}

In this case, it may be cleaner to use approach #1, to avoid the need
for the extra ref/unrefs. In some other cases, e.g. when the reference
of the object returned is owned by some other thing, like a async
method context helper struct, then it would be better to use approach
#2, as that clearly shows who owns each reference.

-- 
Aleksander
https://aleksander.es