After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 642295 - StThemeNode: use (out caller-allocates) on ClutterColor-returning methods
StThemeNode: use (out caller-allocates) on ClutterColor-returning methods
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-02-14 14:26 UTC by Dan Winship
Modified: 2011-02-14 15:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
StThemeNode: use (out caller-allocates) on ClutterColor-returning methods (8.40 KB, patch)
2011-02-14 14:26 UTC, Dan Winship
committed Details | Review

Description Dan Winship 2011-02-14 14:26:15 UTC
While trying to figure out why the use of st_theme_node_get_content_box()
in boxpointer.js (a) had already worked before the patch in bug 642192,
and (b) didn't need to be changed after the patch in bug 642192, I
determined that (a) it didn't truly "work", it just hadn't crashed *yet*,
and (b) we could finally fix all the color-returning methods.

clutter's own color-returning methods are still un-annotated, and I don't
know what their feeling on ABI of bindings is. Of course, we could just
patch over their methods with nice wrappers from environment.js...
Comment 1 Dan Winship 2011-02-14 14:26:17 UTC
Created attachment 180830 [details] [review]
StThemeNode: use (out caller-allocates) on ClutterColor-returning methods

Properly annotate the themenode methods that return ClutterColors, and
update their JS callers to take advantage of that.
Comment 2 Owen Taylor 2011-02-14 14:52:20 UTC
(In reply to comment #0)
> While trying to figure out why the use of st_theme_node_get_content_box()
> in boxpointer.js (a) had already worked before the patch in bug 642192,
> and (b) didn't need to be changed after the patch in bug 642192, I
> determined that (a) it didn't truly "work", it just hadn't crashed *yet*,
> and (b) we could finally fix all the color-returning methods.
> 
> clutter's own color-returning methods are still un-annotated, and I don't
> know what their feeling on ABI of bindings is. Of course, we could just
> patch over their methods with nice wrappers from environment.js...

Clutter seems basically OK. The only two Clutter.Color() returning functions we use that have the mucky old form from JS are clutter_color_from_string() and clutter_color_from_pixel() and they *are* annotated in clutter as (out caller-allocates) - the fact we call them passing in an existing color is a different G-I issue - it should be indentifying "methods" that have an (out) argument as the first argument as static.

Then something like clutter_rectangle_get_color() is annotated as (out) not (out caller-allocates), but is auto-detected by G-I as (out caller-allocates) because G-I is smart enough to realize that a single-dereference structure can't be returning a pointer.
Comment 3 Owen Taylor 2011-02-14 14:53:01 UTC
Review of attachment 180830 [details] [review]:

Looks good
Comment 4 Dan Winship 2011-02-14 15:50:21 UTC
Attachment 180830 [details] pushed as 1224e95 - StThemeNode: use (out caller-allocates) on ClutterColor-returning methods