[gjs] Handle wide ranging enum values better



commit 259ede142fd4dcf7171555aa50e87d2e652d6220
Author: Owen W. Taylor <otaylor fishsoup net>
Date:   Tue Sep 14 15:58:00 2010 -0400

    Handle wide ranging enum values better
    
    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.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=629705

 gi/arg.c                       |   96 +++++++++++++++++++++++++++++++++-------
 gi/arg.h                       |    5 ++-
 gi/enumeration.c               |   12 +++--
 gi/value.c                     |   50 +++++++++++++++++---
 gjs/jsapi-util.c               |   46 +++++++++++++++++++
 gjs/jsapi-util.h               |    4 ++
 test/js/testEverythingBasic.js |   11 ++++-
 7 files changed, 193 insertions(+), 31 deletions(-)
---
diff --git a/gi/arg.c b/gi/arg.c
index b79f743..f4cda1a 100644
--- a/gi/arg.c
+++ b/gi/arg.c
@@ -38,7 +38,7 @@
 JSBool
 _gjs_flags_value_is_valid(JSContext   *context,
                           GType        gtype,
-                          guint        value)
+                          gint64       value)
 {
     GFlagsValue *v;
     guint32 tmpval;
@@ -51,13 +51,20 @@ _gjs_flags_value_is_valid(JSContext   *context,
     klass = g_type_class_ref(gtype);
 
     /* check all bits are defined for flags.. not necessarily desired */
-    tmpval = value;
+    tmpval = (guint32)value;
+    if (tmpval != value) { /* Not a guint32 */
+        gjs_throw(context,
+                  "0x%" G_GINT64_MODIFIER "x is not a valid value for flags %s",
+                  value, g_type_name(G_TYPE_FROM_CLASS(klass)));
+        return JS_FALSE;
+    }
+
     while (tmpval) {
         v = g_flags_get_first_value(klass, tmpval);
         if (!v) {
             gjs_throw(context,
                       "0x%x is not a valid value for flags %s",
-                      value, g_type_name(G_TYPE_FROM_CLASS(klass)));
+                      (guint32)value, g_type_name(G_TYPE_FROM_CLASS(klass)));
             return JS_FALSE;
         }
 
@@ -71,7 +78,7 @@ _gjs_flags_value_is_valid(JSContext   *context,
 static JSBool
 _gjs_enum_value_is_valid(JSContext  *context,
                          GIEnumInfo *enum_info,
-                         int         value)
+                         gint64      value)
 {
     JSBool found;
     int n_values;
@@ -82,7 +89,7 @@ _gjs_enum_value_is_valid(JSContext  *context,
 
     for (i = 0; i < n_values; ++i) {
         GIValueInfo *value_info;
-        long enum_value;
+        gint64 enum_value;
 
         value_info = g_enum_info_get_value(enum_info, i);
         enum_value = g_value_info_get_value(value_info);
@@ -96,13 +103,52 @@ _gjs_enum_value_is_valid(JSContext  *context,
 
     if (!found) {
         gjs_throw(context,
-                  "%d is not a valid value for enumeration %s",
+                  "%" G_GINT64_MODIFIER "d is not a valid value for enumeration %s",
                   value, g_base_info_get_name((GIBaseInfo *)enum_info));
     }
 
     return found;
 }
 
+static gboolean
+_gjs_enum_uses_signed_type (GIEnumInfo *enum_info)
+{
+    switch (g_enum_info_get_storage_type (enum_info)) {
+    case GI_TYPE_TAG_INT8:
+    case GI_TYPE_TAG_INT16:
+    case GI_TYPE_TAG_INT32:
+    case GI_TYPE_TAG_INT64:
+        return TRUE;
+    default:
+        return FALSE;
+    }
+}
+
+/* This is hacky - g_function_info_invoke() and g_field_info_get/set_field() expect
+ * arg->v_int to have the enum value in arg->v_int and depend on all flags and
+ * enumerations being passed on the stack in a 32-bit field. See FIXME comment in
+ * g_field_info_get_field. The same assumption of enums cast to 32-bit signed integers
+ * is found in g_value_set_enum/g_value_set_flags().
+ */
+
+gint64
+_gjs_enum_from_int (GIEnumInfo *enum_info,
+                    int         int_value)
+{
+    if (_gjs_enum_uses_signed_type (enum_info))
+        return (gint64)int_value;
+    else
+        return (gint64)(guint32)int_value;
+}
+
+/* Here for symmetry, but result is the same for the two cases */
+static int
+_gjs_enum_to_int (GIEnumInfo *enum_info,
+                  gint64      value)
+{
+    return (int)value;
+}
+
 /* The typelib used to have machine-independent types like
  * GI_TYPE_TAG_LONG that had to be converted; now we only
  * handle GType specially here.
@@ -982,17 +1028,25 @@ gjs_value_to_g_argument(JSContext      *context,
                 nullable_type = FALSE;
 
                 if (interface_type == GI_INFO_TYPE_ENUM) {
-                    if (!JS_ValueToInt32(context, value, &arg->v_int)) {
+                    gint64 value_int64;
+
+                    if (!gjs_value_to_int64 (context, value, &value_int64))
                         wrong = TRUE;
-                    } else if (!_gjs_enum_value_is_valid(context, (GIEnumInfo *)interface_info, arg->v_int)) {
+                    else if (!_gjs_enum_value_is_valid(context, (GIEnumInfo *)interface_info, value_int64))
                         wrong = TRUE;
-                    }
+                    else
+                        arg->v_int = _gjs_enum_to_int ((GIEnumInfo *)interface_info, value_int64);
+
                 } else if (interface_type == GI_INFO_TYPE_FLAGS) {
-                    if (!JS_ValueToInt32(context, value, &arg->v_int)) {
+                    gint64 value_int64;
+
+                    if (!gjs_value_to_int64 (context, value, &value_int64))
                         wrong = TRUE;
-                    } else if (!_gjs_flags_value_is_valid(context, gtype, arg->v_int)) {
+                    else if (!_gjs_flags_value_is_valid(context, gtype, value_int64))
                         wrong = TRUE;
-                    }
+                    else
+                        arg->v_int = _gjs_enum_to_int ((GIEnumInfo *)interface_info, value_int64);
+
                 } else if (gtype == G_TYPE_NONE) {
                     gjs_throw(context, "Unexpected unregistered type unpacking GArgument from Number");
                     wrong = TRUE;
@@ -1720,14 +1774,24 @@ gjs_value_from_g_argument (JSContext  *context,
 
             /* Enum/Flags are aren't pointer types, unlike the other interface subtypes */
             if (interface_type == GI_INFO_TYPE_ENUM) {
-                if (_gjs_enum_value_is_valid(context, (GIEnumInfo *)interface_info, arg->v_int))
-                    value = INT_TO_JSVAL(arg->v_int);
+                gint64 value_int64 = _gjs_enum_from_int ((GIEnumInfo *)interface_info, arg->v_int);
+
+                if (_gjs_enum_value_is_valid(context, (GIEnumInfo *)interface_info, value_int64)) {
+                    jsval tmp;
+                    if (JS_NewNumberValue(context, value_int64, &tmp))
+                        value = tmp;
+                }
 
                 goto out;
             } else if (interface_type == GI_INFO_TYPE_FLAGS) {
+                gint64 value_int64 = _gjs_enum_from_int ((GIEnumInfo *)interface_info, arg->v_int);
+
                 gtype = g_registered_type_info_get_g_type((GIRegisteredTypeInfo*)interface_info);
-                if (_gjs_flags_value_is_valid(context, gtype, arg->v_int))
-                    value = INT_TO_JSVAL(arg->v_int);
+                if (_gjs_flags_value_is_valid(context, gtype, value_int64)) {
+                    jsval tmp;
+                    if (JS_NewNumberValue(context, value_int64, &tmp))
+                        value = tmp;
+                }
 
                 goto out;
             } else if (interface_type == GI_INFO_TYPE_STRUCT &&
diff --git a/gi/arg.h b/gi/arg.h
index d4a2d03..df473e2 100644
--- a/gi/arg.h
+++ b/gi/arg.h
@@ -74,7 +74,10 @@ JSBool gjs_g_argument_release_in_arg (JSContext  *context,
 
 JSBool _gjs_flags_value_is_valid (JSContext   *context,
                                   GType        gtype,
-                                  guint        value);
+                                  gint64       value);
+
+gint64 _gjs_enum_from_int (GIEnumInfo *enum_info,
+                           int         int_value);
 
 JSBool gjs_array_from_strv (JSContext   *context,
                             jsval       *value_p,
diff --git a/gi/enumeration.c b/gi/enumeration.c
index 09f166c..fadb046 100644
--- a/gi/enumeration.c
+++ b/gi/enumeration.c
@@ -64,10 +64,11 @@ gjs_define_enum_value(JSContext    *context,
     const char *value_name;
     char *fixed_name;
     gsize i;
-    int value_val;
+    gint64 value_val;
+    jsval value_js;
 
     value_name = g_base_info_get_name( (GIBaseInfo*) info);
-    value_val = (int) g_value_info_get_value(info);
+    value_val = g_value_info_get_value(info);
 
     /* g-i converts enum members such as GDK_GRAVITY_SOUTH_WEST to
      * Gdk.GravityType.south-west (where 'south-west' is value_name)
@@ -82,11 +83,12 @@ gjs_define_enum_value(JSContext    *context,
     }
 
     gjs_debug(GJS_DEBUG_GENUM,
-              "Defining enum value %s (fixed from %s) %d",
+              "Defining enum value %s (fixed from %s) %" G_GINT64_MODIFIER "d",
               fixed_name, value_name, value_val);
 
-    if (!JS_DefineProperty(context, in_object,
-                           fixed_name, INT_TO_JSVAL(value_val),
+    if (!JS_NewNumberValue(context, value_val, &value_js) ||
+        !JS_DefineProperty(context, in_object,
+                           fixed_name, value_js,
                            NULL, NULL,
                            GJS_MODULE_PROP_FLAGS)) {
         gjs_throw(context, "Unable to define enumeration value %s %d (no memory most likely)",
diff --git a/gi/value.c b/gi/value.c
index a180cdb..28b8d3d 100644
--- a/gi/value.c
+++ b/gi/value.c
@@ -404,11 +404,14 @@ gjs_value_to_g_value_internal(JSContext    *context,
         else
             g_value_set_boxed(gvalue, gboxed);
     } else if (g_type_is_a(gtype, G_TYPE_ENUM)) {
-        if (JSVAL_IS_INT(value)) {
+        gint64 value_int64;
+
+        if (gjs_value_to_int64 (context, value, &value_int64)) {
             GEnumValue *v;
 
+            /* See arg.c:_gjs_enum_to_int() */
             v = g_enum_get_value(G_ENUM_CLASS(g_type_class_peek(gtype)),
-                                 JSVAL_TO_INT(value));
+                                 (int)value_int64);
             if (v == NULL) {
                 gjs_throw(context,
                           "%d is not a valid value for enumeration %s",
@@ -425,11 +428,14 @@ gjs_value_to_g_value_internal(JSContext    *context,
             return JS_FALSE;
         }
     } else if (g_type_is_a(gtype, G_TYPE_FLAGS)) {
-        if (JSVAL_IS_INT(value)) {
-            if (!_gjs_flags_value_is_valid(context, gtype, JSVAL_TO_INT(value)))
+        gint64 value_int64;
+
+        if (gjs_value_to_int64 (context, value, &value_int64)) {
+            if (!_gjs_flags_value_is_valid(context, gtype, value_int64))
                 return JS_FALSE;
 
-            g_value_set_flags(gvalue, value);
+            /* See arg.c:_gjs_enum_to_int() */
+            g_value_set_flags(gvalue, (int)value_int64);
         } else {
             gjs_throw(context,
                       "Wrong type %s; flags %s expected",
@@ -515,6 +521,36 @@ gjs_value_to_g_value_no_copy(JSContext    *context,
 }
 
 static JSBool
+convert_int_to_enum (JSContext *context,
+                     jsval     *value_p,
+                     GType      gtype,
+                     int        v)
+{
+    double v_double;
+
+    if (v > 0 && v < G_MAXINT) {
+        /* Optimize the unambiguous case */
+        v_double = v;
+    } else {
+        GIBaseInfo *info;
+
+        /* Need to distinguish between negative integers and unsigned integers */
+
+        info = g_irepository_find_by_gtype(g_irepository_get_default(),
+                                           gtype);
+
+        if (info == NULL) /* hope for the best */
+            v_double = v;
+        else
+            v_double = _gjs_enum_from_int ((GIEnumInfo *)info, v);
+
+        g_base_info_unref(info);
+    }
+
+    return JS_NewNumberValue(context, v_double, value_p);
+}
+
+static JSBool
 gjs_value_from_g_value_internal(JSContext    *context,
                                 jsval        *value_p,
                                 const GValue *gvalue,
@@ -630,9 +666,7 @@ gjs_value_from_g_value_internal(JSContext    *context,
         }
         *value_p = OBJECT_TO_JSVAL(obj);
     } else if (g_type_is_a(gtype, G_TYPE_ENUM)) {
-        int v;
-        v = g_value_get_enum(gvalue);
-        *value_p = INT_TO_JSVAL(v);
+        return convert_int_to_enum(context, value_p, gtype, g_value_get_enum(gvalue));
     } else if (g_type_is_a(gtype, G_TYPE_PARAM)) {
         GParamSpec *gparam;
         JSObject *obj;
diff --git a/gjs/jsapi-util.c b/gjs/jsapi-util.c
index 56e3a3f..d94614c 100644
--- a/gjs/jsapi-util.c
+++ b/gjs/jsapi-util.c
@@ -1270,6 +1270,52 @@ gjs_date_from_time_t (JSContext *context, time_t time)
 }
 
 /**
+ * gjs_value_to_int64:
+ * @context: the Javascript context object
+ * @val: Javascript value to convert
+ * @gint64: location to store the return value
+ *
+ * Converts a Javascript value into the nearest 64 bit signed value.
+ *
+ * This function behaves indentically for rounding to JSValToInt32(), which
+ * means that it rounds (0.5 toward positive infinity) rather than doing
+ * a C-style truncation to 0. If we change to using JSValToEcmaInt32() then
+ * this should be changed to match.
+ *
+ * Return value: If the javascript value converted to a number (see
+ *   JS_ValueToNumber()) is NaN, or outside the range of 64-bit signed
+ *   numbers, fails and sets an exception. Otherwise returns the value
+ *   rounded to the nearest 64-bit integer. Like JS_ValueToInt32(),
+ *   undefined throws, but null => 0, false => 0, true => 1.
+ */
+JSBool
+gjs_value_to_int64  (JSContext  *context,
+                     const jsval val,
+                     gint64     *result)
+{
+    if (JSVAL_IS_INT (val)) {
+        *result = JSVAL_TO_INT (val);
+        return JS_TRUE;
+    } else {
+        double value_double;
+        if (!JS_ValueToNumber(context, val, &value_double))
+            return JS_FALSE;
+
+        if (isnan(value_double) ||
+            value_double < G_MININT64 ||
+            value_double > G_MAXINT64) {
+
+            gjs_throw(context,
+                      "Value is not a valid 64-bit integer");
+            return JS_FALSE;
+        }
+
+        *result = (gint64)(value_double + 0.5);
+        return JS_TRUE;
+    }
+}
+
+/**
  * gjs_parse_args:
  * @context:
  * @function_name: The name of the function being called
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index e75561e..5129b24 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -336,6 +336,10 @@ gboolean    gjs_unichar_from_string          (JSContext       *context,
 
 const char* gjs_get_type_name                (jsval            value);
 
+JSBool      gjs_value_to_int64               (JSContext       *context,
+                                              const jsval      val,
+                                              gint64          *result);
+
 jsval       gjs_date_from_time_t             (JSContext *context, time_t time);
 
 JSBool      gjs_parse_args                   (JSContext  *context,
diff --git a/test/js/testEverythingBasic.js b/test/js/testEverythingBasic.js
index 79dcb34..5f3d39e 100644
--- a/test/js/testEverythingBasic.js
+++ b/test/js/testEverythingBasic.js
@@ -340,8 +340,17 @@ function testNestedGHashOut() {
 
 /* Enums */
 function testEnumParam() {
-   let e = Everything.test_enum_param(Everything.TestEnum.VALUE1);
+   let e;
+
+   e = Everything.test_enum_param(Everything.TestEnum.VALUE1);
+   assertEquals('Enum parameter', 'value1', e);
+   e = Everything.test_enum_param(Everything.TestEnum.VALUE3);
+   assertEquals('Enum parameter', 'value3', e);
+
+   e = Everything.test_unsigned_enum_param(Everything.TestEnumUnsigned.VALUE1);
    assertEquals('Enum parameter', 'value1', e);
+   e = Everything.test_unsigned_enum_param(Everything.TestEnumUnsigned.VALUE2);
+   assertEquals('Enum parameter', 'value2', e);
 }
 
 function testSignal() {



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]