[Webkit-unassigned] [Bug 195574] [GLib] Returning G_TYPE_OBJECT from a method does not work

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=195574

--- Comment #8 from Adrian Perez <aperez at igalia.com> ---
(In reply to Carlos Garcia Campos from comment #7)
> (In reply to Adrian Perez from comment #6)
> > Comment on attachment 364525 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=364525&action=review
> > 
> > Informally reviewing…
> 
> Thanks!
> 
> > There is a couple of places where I think the wording
> > could be improved to avoid ambiguity. Otherwise the added text is a welcome
> > improvement 👍
> > 
> > > Source/JavaScriptCore/API/glib/JSCClass.cpp:593
> > > + * passed to jsc_context_register_class().
> > 
> > Using “freed with” here does not make 100% clear that the GDestroyNotify 
> > supplied by the user of the API is responsible to free the value.
> 
> No? What do you understand then?

Using “freed with” can be parsed as “freed [alongside] with”, which to me
means that the GDestroyNotify will be called, but the freeing will happen
automatically, after the GDestroyNotify callback returns (i.e. the callback
notifies that the item will be freed, but does not need itself to do the
freeing).

If you just change the “with” preposition and use “by” instead, as in “freed
by the GDestroyNotify callback”, then it's clear that freeing has to be done
“by” the callback itself.

> > How about
> > writing:
> > 
> >   Note  that the value returned by @callback is adopted by @jsc_class, and
> >   must be freed by the #GDestroyNotify passed to
> > jsc_context_register_class().
> > 
> > WDYT?
> 
> I'm not sure, the "must" here sounds to me like the user should free the
> instance, while it will be freed by the JSCClass using the destroy function
> passed on class registration.

But the user has to “tell” how to free the data after all by passing a suitable
GDestroyNotify that will free the data… so yes, the user “has to” control how
the data will be freed.

Maybe it is even clearer by writing instead:

  Note that the value returned by @callback is adopted by @jsc_class,
  and the #GDestroyNotify passed to jsc_context_register_class() is
  responsible for disposing of it.

Does this sound any better to you?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190314/08265bc5/attachment.html>