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 618195 - Skip user_data arguments in gjs_callback_closure
Skip user_data arguments in gjs_callback_closure
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2010-05-09 19:38 UTC by Owen Taylor
Modified: 2010-05-13 14:13 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Skip user_data arguments in gjs_callback_closure (2.22 KB, patch)
2010-05-09 19:38 UTC, Owen Taylor 		accepted-commit_now Details | Review
Skip user_data arguments in gjs_callback_closure (2.54 KB, patch)
2010-05-10 15:12 UTC, Owen Taylor 		reviewed Details | Review
Skip user_data arguments in gjs_callback_closure (3.73 KB, patch)
2010-05-10 15:37 UTC, Owen Taylor 		committed Details | Review

Description Owen Taylor 2010-05-09 19:38:42 UTC 
The user_data argument to a callback now holds the GjsCallbackTrampoline
instead of a JS provided value, so we must not cast it to a jsval
and treat it as a function argument.

This fixes segfaults when trying to get a stracktrace inside a callback.
Comment 1 Owen Taylor 2010-05-09 19:38:44 UTC 
Created attachment 160663 [details] [review]
Skip user_data arguments in gjs_callback_closure
Comment 2 Colin Walters 2010-05-10 14:41:36 UTC 
Review of attachment 160663 [details] [review]:

Oops, good catch.
Comment 3 Owen Taylor 2010-05-10 15:12:58 UTC 
Created attachment 160728 [details] [review]
Skip user_data arguments in gjs_callback_closure

New version that passes the right argc to JS_CallFunctionValue
Comment 4 Johan (not receiving bugmail) Dahlin 2010-05-10 15:20:00 UTC 
Review of attachment 160728 [details] [review]:

Looks good. Should update tests to verify that arguments.length for callbacks
is correctly set.

::: gi/function.c
@@ -159,3 @@
     g_assert(n_args >= 0);
 
     jsargs = (jsval*)g_newa(jsval, n_args);

You're allocating space for the void arguments here as well, afacs.
Comment 5 Owen Taylor 2010-05-10 15:37:16 UTC 
Created attachment 160729 [details] [review]
Skip user_data arguments in gjs_callback_closure

This adds a test (not *quite* sure I understand the point of the three
argument form of assertEquals()). 

The over-allocation of jsargs is intentional - it's not worth two
passes to avoid allocating a single jsval on the stack.
Comment 6 Johan (not receiving bugmail) Dahlin 2010-05-13 14:11:37 UTC 
Review of attachment 160729 [details] [review]:

Looks great, thanks.
Comment 7 Owen Taylor 2010-05-13 14:13:15 UTC 
Attachment 160729 [details] pushed as 269e4fd - Skip user_data arguments in gjs_callback_closure