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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Mar 14 06:00:59 PDT 2019


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

--- Comment #7 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(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?

> 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.

> > Source/JavaScriptCore/API/glib/JSCValue.cpp:594
> > + * When @instance is provided, @jsc_class must be provided too. @jscClass takes ownership of
> 
> @jscClass → @jsc_class

Oops

> > Source/JavaScriptCore/API/glib/JSCValue.cpp:595
> > + * @instance that will be freed with the #GDestroyNotify passed to jsc_context_register_class().
> 
> How about the same as above?
> 
>   …@instance must be freed by the #GDestroyNotify passed to
> jsc_context_register_class().

-- 
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/ce81a3ad/attachment.html>


More information about the webkit-unassigned mailing list