GNOME Bugzilla – Bug 640468
support fixed size array as return value && make struct with array inside direct allocatiable
Last modified: 2011-06-04 22:04:26 UTC
Created attachment 179225 [details] [review] g_field_info_get_field: return correct pointer for C array .
Created attachment 179226 [details] [review] support fixed size array as return value
Created attachment 179227 [details] [review] make struct with array inside direct allocatiable
Review of attachment 179225 [details] [review]: I'm not an expert as that code but it looks good. I assume this is tested in gjs. I think you need to update the tests. did you run make check?
Created attachment 181170 [details] [review] g_field_info_get_field: return correct pointer for C array > this is tested in gjs. right. > I think you need to update the tests. done. > did you run make check? Sorry, forgot about it
Ping. It would be great to see this go in if it is blocking bug 634080 (required support for a system monitor applet)
Will look at this after 3.0; my initial reaction though is that I'd like to avoid people using complex raw structures. For example, it might be better to invest time in having a good GVariant binding, as a general purpose readonly data passing tool.
(In reply to comment #6) > Will look at this after 3.0; my initial reaction though is that I'd like to > avoid people using complex raw structures. Now that 3.0 has shipped, any word on this? As John pointed out, what's motivating this is that we'd like a system monitor applet for GNOME Shell. Specifically, we want to be able to call glibtop_get_cpu() from JavaScript. That function takes a pointer to a struct glibtop_cpu, which includes a few embedded fixed-size arrays of ints; gjs can't currently handle that. You said you might prefer a GVariant binding instead. I'm not sure I understand completely - would that let JavaScript call this same function, for example?
Review of attachment 179226 [details] [review]: Otherwise looks OK. ::: gi/arg.c @@ +1577,3 @@ + +finally: + case GI_TYPE_TAG_UINT16: There is no corresponding JS_AddValueRoot for this. Personally I'd say just remove it - we will shortly hard require JS 1.8.5.
Review of attachment 179227 [details] [review]: The patch looks OK, but can you get the test case working before committing? ::: gi/boxed.c @@ +1027,2 @@ static gboolean +type_is_direct_allocatiable (GITypeInfo *type_info) Hm, how about "type_can_be_allocated_directly" ? "alloctable" is a weird word. ::: test/js/testEverythingEncapsulated.js @@ +139,3 @@ +function testTestDirectAllocatiable() { + let struct = new Everything.TestDirectAllocatiable; Missing parenthesis here? let struct = new Everything.TestDirectAllocatiable(); Actually this looks like a typo...I don't even see a "TestDirect" in the g-i sources. Is there another patch?
Created attachment 186990 [details] [review] support fixed size array as return value > There is no corresponding JS_AddValueRoot for this. added corresponding JS_AddValueRoot
Created attachment 186991 [details] [review] struct with array inside can be allocated directly > Hm, how about "type_can_be_allocated_directly" ? renamed > I don't even see a "TestDirect" in the g-i sources. Is there another patch? yes. It attached to this bug. > The patch looks OK, but can you get the test case working before committing? It pass it, but require patched g-i.
Comment on attachment 181170 [details] [review] g_field_info_get_field: return correct pointer for C array Attachment 181170 [details] pushed as 57c51b7 - g_field_info_get_field: return correct pointer for C array
Comment on attachment 186991 [details] [review] struct with array inside can be allocated directly Attachment 186991 [details] pushed as 943f958 - struct with array inside can be allocated directly
Attachment 186990 [details] pushed as 5ede703 - support fixed size array as return value
Hi there, So using gjs commit 39cf5d2f51dcad72a7e175f21a3ffe602b5dd4b3 (HEAD at the moment), I've found that I can't access the fixed size array elements. Adding the following into the system-monitor gnome-shell extension (in CpuIndicator._initValues just after this.prev has been populated by GTop.glibtop_get_cpu) global.log(this._prev.xcpu_total[0]); causes it to crash gnome-shell with no javascript error, or any other kind of warning. My .xsession-errors file contains a HUP for gnome-shell-calendar-server and some warnings from gnome-session about gnome-shell being "killed by signal", but nothing directly from gnome-shell. I don't really know how to debug this any further, but I'd be happy to run further tests and provide information if I can be of help? Similarly, if this should be filed under a different bug, please let me know and I'll open a new one...
(In reply to comment #15) > I've found that I can't access the fixed size array elements. Adding > the following into the system-monitor gnome-shell extension (in > CpuIndicator._initValues just after this.prev has been populated by > GTop.glibtop_get_cpu) > > global.log(this._prev.xcpu_total[0]); work fine here. If you can reproduce it, please create new bug and attach stacktrace there. > I don't really know how to debug this any further http://live.gnome.org/GnomeShell/Debugging
Thanks very much Maxim, I had some difficulty getting gnome-shell debugging using the instructions, but eventually managed it through gdbserver. I've filed bug 651892 with a backtrace for those that are interested...