GNOME Bugzilla – Bug 629705
Handle wide ranging enum values better
Last modified: 2011-01-13 01:15:27 UTC
Enums were being converted to and from jsval using JSVAL_TO_INT and INT_TO_JSVAL. The latter is just wrong - INT_TO_JSVAL produces junk results if called on an integer outside the range of -0x40000000 - 0x3fffffff. Just fixing that still leaves a problem - if we get 0x80000000 as a flags value, and we treat it as (int)0x80000000 then we pass it to another function that is using 'guint' for flags instead of the enum type, we'll throw because the value is out of range. So, this change goes a further and (by depending on gobject-introspection changes in 629704) correctly maps all signed and unsigned 32 bit constants onto the correct numeric value. A new utility function gjs_value_to_int64() is added to convert from jsval to gint64, since we are using int64 as an intermediate type for holding enum values.
Created attachment 170284 [details] [review] Handle wide ranging enum values better
Review of attachment 170284 [details] [review]: Two minor comments. ::: gi/arg.c @@ +52,2 @@ /* check all bits are defined for flags.. not necessarily desired */ tmpval = value; Include a cast now to avoid a compiler warning? ::: test/js/testEverythingBasic.js @@ +352,3 @@ assertEquals('Enum parameter', 'value1', e); + e = Everything.test_unsigned_enum_param(Everything.TestEnumUnsigned.VALUE2); + log(e); You told me not to add log() to the test suite earlier, so... =)
(In reply to comment #2) > Review of attachment 170284 [details] [review]: > > Two minor comments. > > ::: gi/arg.c > @@ +52,2 @@ > /* check all bits are defined for flags.. not necessarily desired */ > tmpval = value; > > Include a cast now to avoid a compiler warning? I don't get a warning, but sure, doesn't hurt. > ::: test/js/testEverythingBasic.js > @@ +352,3 @@ > assertEquals('Enum parameter', 'value1', e); > + e = > Everything.test_unsigned_enum_param(Everything.TestEnumUnsigned.VALUE2); > + log(e); > > You told me not to add log() to the test suite earlier, so... =) Yeah, left-over
Attachment 170284 [details] pushed as 259ede1 - Handle wide ranging enum values better