<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Dec 9, 2013 at 4:26 AM, Aleksander Morgado <span dir="ltr"><<a href="mailto:aleksander@lanedo.com" target="_blank">aleksander@lanedo.com</a>></span> wrote:<br>


<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><span style="color:rgb(34,34,34)">One comment here; when setting the result in a GSimpleAsyncResult, you</span><br>


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


</div><div><br></div><div>Thanks,</div><div>Ben</div></div></div></div>