[PATCH] sim-mbim: implement operator identifier and name loading

[Dan Williams]

On Mon, 2013-12-09 at 10:33 -0800, Ben Chan wrote:
> On Mon, Dec 9, 2013 at 4:26 AM, Aleksander Morgado <aleksander at lanedo.com>wrote:
> 
> > One comment here; when setting the result in a GSimpleAsyncResult, you
> >  need to make sure that the ownership is fully managed in every case. In
> > the case above you're setting a heap-allocated string as a result,
> > giving a NULL GDestroyNotify, because you know that _finish() will take
> > that string and return it. But that is not really ok, as _finish() may
> > never be called if the caller of the async method didn't provide a
> > GAsyncReadyCallback. In other words, if I run:
> >
> >   load_operator_identifier (sim, NULL, NULL);
> >
> > I would be leaking that string that was set in the GSimpleAsyncResult.
> >
> >
> > To manage this situation properly, you can either:
> >
> > 1) Don't set a newly allocated string as result in the
> > GSimpleAsyncResult. In your example, you could have done:
> >
> >   g_simple_async_result_set_op_res_gpointer (simple,
> >                                              provider->provider_id,
> >                                              NULL);
> >   g_simple_async_result_complete (simple);
> >   mbim_provider_free (provider);
> >
> > As you are completing inline (not in idle), the string that you did set
> > as result will be valid if _finish() is being called. So then you can
> > g_strdup() within _finish(), only if it was ever called.
> >
> > 2) Set a newly allocated string as result, and provide g_free as
> > GDestroyNotify. This will then force you to g_strdup() again during
> > _finish(), so you would be allocating twice, but this will make it
> > possible to complete_in_idle().
> >
> > The new GTask API in newer glib manages this logic much better, as it
> > allows you to 'steal' the result from the GSimpleAsyncResult (IIRC).
> >
> >
> >
> > For this specific case it probably is not a big deal, because I'm not
> > sure that we have such calls passing NULL for GAsyncReadyCallback, but
> > anyway, I would fix those (e.g. to use approach #1) just for completeness.
> >
> 
> That makes sense. I'll prepare a patch. I followed load_sim_identifier and
> load_imsi. We should probably fix them as well.

Thanks, I'll review & apply once they arrive if Aleksander hasn't done
that already.

Dan