GNOME.org
 

[pygobject] Fix signedness, overflow checking, and 32 bit overflow of GFlags

commit caeeeb7e4282e183eefc3c53b2d53c8c2bb7de89
Author: Martin Pitt <martinpitt gnome org>
Date:   Wed Feb 27 08:07:20 2013 +0100

    Fix signedness, overflow checking, and 32 bit overflow of GFlags
    
    GFlagsValue.value is a guint, so we must access it as unsigned type. Define two
    new macros PYGLIB_PyLong_FromUnsignedLong() and PYGLIB_PyLong_AsUnsignedLong()
    for that purpose, and consistently use them for handling flag values. Use the
    checked variant of these functions which produce OverflowErrors instead
    of the unchecked PYGLIB_PyLong_AS_LONG().
    
    Insert zero padding after the PyLongObject in PyGFlags and PyGEnum. Without
    this, the directly adjacent GType field seems to confuse
    PyLong_FromUnsignedLong() and includes the GType into the numeric value.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=693121

 gi/_glib/pyglib-python-compat.h |    8 ++++++
 gi/_gobject/gobjectmodule.c     |    2 +-
 gi/_gobject/pygflags.c          |   52 ++++++++++++++++++++++++---------------
 gi/_gobject/pygobject-private.h |   10 ++++---
 gi/_gobject/pygobject.h         |    4 +-
 gi/_gobject/pygtype.c           |    6 ++--
 tests/test_overrides_gdk.py     |   15 +++++++++++
 7 files changed, 67 insertions(+), 30 deletions(-)
---
diff --git a/gi/_glib/pyglib-python-compat.h b/gi/_glib/pyglib-python-compat.h
index 96a20c0..9ba68f7 100644
--- a/gi/_glib/pyglib-python-compat.h
+++ b/gi/_glib/pyglib-python-compat.h
@@ -137,6 +137,11 @@ static int _pyglib_init_##modname(PyObject *module)
 #define PYGLIB_PyLong_AS_LONG PyInt_AS_LONG
 #define Py_TYPE(ob) (((PyObject*)(ob))->ob_type)
 
+/* Python 2.7 lacks a PyInt_FromUnsignedLong function; use signed longs, and
+ * rely on PyInt_AsUnsignedLong() to interpret them correctly */
+#define PYGLIB_PyLong_FromUnsignedLong PyInt_FromLong
+#define PYGLIB_PyLong_AsUnsignedLong(o) PyInt_AsUnsignedLongMask((PyObject*)(o))
+
 #define PYGLIB_PyNumber_Long PyNumber_Int
 
 #ifndef PyVarObject_HEAD_INIT
@@ -225,6 +230,9 @@ PyTypeObject symbol = {                                 \
 #define PYGLIB_PyLongObject PyLongObject
 #define PYGLIB_PyLong_Type PyLong_Type
 
+#define PYGLIB_PyLong_FromUnsignedLong PyLong_FromUnsignedLong
+#define PYGLIB_PyLong_AsUnsignedLong(o) PyLong_AsUnsignedLongMask((PyObject*)(o))
+
 #define PYGLIB_PyBytes_FromString PyBytes_FromString
 #define PYGLIB_PyBytes_FromStringAndSize PyBytes_FromStringAndSize
 #define PYGLIB_PyBytes_Resize(o, len) _PyBytes_Resize(o, len)
diff --git a/gi/_gobject/gobjectmodule.c b/gi/_gobject/gobjectmodule.c
index 9fb7564..c49b412 100644
--- a/gi/_gobject/gobjectmodule.c
+++ b/gi/_gobject/gobjectmodule.c
@@ -572,7 +572,7 @@ create_property (const gchar  *prop_name,
                return NULL;
 
            if (pyg_flags_get_value(prop_type, pydefault,
-                                   (gint *)&default_value))
+                                   &default_value))
                return NULL;
 
            pspec = g_param_spec_flags (prop_name, nick, blurb,
diff --git a/gi/_gobject/pygflags.c b/gi/_gobject/pygflags.c
index bad32c6..bb12d4a 100644
--- a/gi/_gobject/pygflags.c
+++ b/gi/_gobject/pygflags.c
@@ -3,7 +3,7 @@
  * Copyright (C) 1998-2003  James Henstridge
  * Copyright (C) 2004       Johan Dahlin
  *
- *   pygenum.c: GFlags wrapper
+ *   pygflags.c: GFlags wrapper
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -40,11 +40,12 @@ pyg_flags_val_new(PyObject* subclass, GType gtype, PyObject *intval)
 {
     PyObject *args, *item;
     args = Py_BuildValue("(O)", intval);
-    item =  (&PYGLIB_PyLong_Type)->tp_new((PyTypeObject*)subclass, args, NULL);
+    g_assert(PyObject_IsSubclass(subclass, (PyObject*) &PyGFlags_Type));
+    item = PYGLIB_PyLong_Type.tp_new((PyTypeObject*)subclass, args, NULL);
     Py_DECREF(args);
     if (!item)
        return NULL;
-    ((PyGEnum*)item)->gtype = gtype;
+    ((PyGFlags*)item)->gtype = gtype;
 
     return item;
 }
@@ -70,7 +71,7 @@ pyg_flags_richcompare(PyGFlags *self, PyObject *other, int op)
 }
 
 static char *
-generate_repr(GType gtype, int value)
+generate_repr(GType gtype, guint value)
 {
     GFlagsClass *flags_class;
     char *retval = NULL, *tmp;
@@ -108,13 +109,13 @@ pyg_flags_repr(PyGFlags *self)
     char *tmp, *retval;
     PyObject *pyretval;
 
-    tmp = generate_repr(self->gtype, PYGLIB_PyLong_AS_LONG(self));
+    tmp = generate_repr(self->gtype, PYGLIB_PyLong_AsUnsignedLong(self));
 
     if (tmp)
         retval = g_strdup_printf("<flags %s of type %s>", tmp,
                                  g_type_name(self->gtype));
     else
-        retval = g_strdup_printf("<flags %ld of type %s>", PYGLIB_PyLong_AS_LONG(self),
+        retval = g_strdup_printf("<flags %ld of type %s>", PYGLIB_PyLong_AsUnsignedLong(self),
                                  g_type_name(self->gtype));
     g_free(tmp);
 
@@ -128,7 +129,7 @@ static PyObject *
 pyg_flags_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
 {
     static char *kwlist[] = { "value", NULL };
-    long value;
+    guint value;
     PyObject *pytc, *values, *ret, *pyint;
     GType gtype;
     GFlagsClass *eclass;
@@ -167,7 +168,7 @@ pyg_flags_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
 
     g_type_class_unref(eclass);
 
-    pyint = PYGLIB_PyLong_FromLong(value);
+    pyint = PYGLIB_PyLong_FromUnsignedLong(value);
     ret = PyDict_GetItem(values, pyint);
     if (!ret) {
         PyErr_Clear();
@@ -185,10 +186,13 @@ pyg_flags_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
 }
 
 PyObject*
-pyg_flags_from_gtype (GType gtype, int value)
+pyg_flags_from_gtype (GType gtype, guint value)
 {
     PyObject *pyclass, *values, *retval, *pyint;
 
+    if (PyErr_Occurred())
+        return PYGLIB_PyLong_FromUnsignedLong(0);
+
     g_return_val_if_fail(gtype != G_TYPE_INVALID, NULL);
 
     /* Get a wrapper class by:
@@ -202,11 +206,11 @@ pyg_flags_from_gtype (GType gtype, int value)
     if (!pyclass)
         pyclass = pyg_flags_add(NULL, g_type_name(gtype), NULL, gtype);
     if (!pyclass)
-       return PYGLIB_PyLong_FromLong(value);
+       return PYGLIB_PyLong_FromUnsignedLong(value);
 
     values = PyDict_GetItemString(((PyTypeObject *)pyclass)->tp_dict,
                                  "__flags_values__");
-    pyint = PYGLIB_PyLong_FromLong(value);
+    pyint = PYGLIB_PyLong_FromUnsignedLong(value);
     retval = PyDict_GetItem(values, pyint);
     if (!retval) {
        PyErr_Clear();
@@ -221,6 +225,10 @@ pyg_flags_from_gtype (GType gtype, int value)
     return retval;
 }
 
+/*
+ * pyg_flags_add
+ * Dynamically create a class derived from PyGFlags based on the given GType.
+ */
 PyObject *
 pyg_flags_add (PyObject *   module,
               const char * typename,
@@ -241,13 +249,16 @@ pyg_flags_add (PyObject *   module,
 
     state = pyglib_gil_state_ensure();
 
+    /* Create a new type derived from GFlags. This is the same as:
+     * >>> stub = type(typename, (GFlags,), {})
+     */
     instance_dict = PyDict_New();
     stub = PyObject_CallFunction((PyObject *)&PyType_Type, "s(O)O",
                                  typename, (PyObject *)&PyGFlags_Type,
                                  instance_dict);
     Py_DECREF(instance_dict);
     if (!stub) {
-       PyErr_SetString(PyExc_RuntimeError, "can't create const");
+       PyErr_SetString(PyExc_RuntimeError, "can't create GFlags subtype");
        pyglib_gil_state_release(state);
         return NULL;
     }
@@ -277,7 +288,8 @@ pyg_flags_add (PyObject *   module,
     for (i = 0; i < eclass->n_values; i++) {
       PyObject *item, *intval;
       
-      intval = PYGLIB_PyLong_FromLong(eclass->values[i].value);
+      intval = PYGLIB_PyLong_FromUnsignedLong(eclass->values[i].value);
+      g_assert(PyErr_Occurred() == NULL);
       item = pyg_flags_val_new(stub, gtype, intval);
       PyDict_SetItem(values, intval, item);
       Py_DECREF(intval);
@@ -312,7 +324,7 @@ pyg_flags_and(PyGFlags *a, PyGFlags *b)
                                                       (PyObject*)b);
 
        return pyg_flags_from_gtype(a->gtype,
-                                   PYGLIB_PyLong_AS_LONG(a) & PYGLIB_PyLong_AS_LONG(b));
+                                   PYGLIB_PyLong_AsUnsignedLong(a) & PYGLIB_PyLong_AsUnsignedLong(b));
 }
 
 static PyObject *
@@ -322,7 +334,7 @@ pyg_flags_or(PyGFlags *a, PyGFlags *b)
                return PYGLIB_PyLong_Type.tp_as_number->nb_or((PyObject*)a,
                                                      (PyObject*)b);
 
-       return pyg_flags_from_gtype(a->gtype, PYGLIB_PyLong_AS_LONG(a) | PYGLIB_PyLong_AS_LONG(b));
+       return pyg_flags_from_gtype(a->gtype, PYGLIB_PyLong_AsUnsignedLong(a) | 
PYGLIB_PyLong_AsUnsignedLong(b));
 }
 
 static PyObject *
@@ -333,7 +345,7 @@ pyg_flags_xor(PyGFlags *a, PyGFlags *b)
                                                       (PyObject*)b);
 
        return pyg_flags_from_gtype(a->gtype,
-                                   PYGLIB_PyLong_AS_LONG(a) ^ PYGLIB_PyLong_AS_LONG(b));
+                                   PYGLIB_PyLong_AsUnsignedLong(a) ^ PYGLIB_PyLong_AsUnsignedLong(b));
 
 }
 
@@ -356,7 +368,7 @@ pyg_flags_get_first_value_name(PyGFlags *self, void *closure)
 
   flags_class = g_type_class_ref(self->gtype);
   g_assert(G_IS_FLAGS_CLASS(flags_class));
-  flags_value = g_flags_get_first_value(flags_class, PYGLIB_PyLong_AS_LONG(self));
+  flags_value = g_flags_get_first_value(flags_class, PYGLIB_PyLong_AsUnsignedLong(self));
   if (flags_value)
       retval = PYGLIB_PyUnicode_FromString(flags_value->value_name);
   else {
@@ -378,7 +390,7 @@ pyg_flags_get_first_value_nick(PyGFlags *self, void *closure)
   flags_class = g_type_class_ref(self->gtype);
   g_assert(G_IS_FLAGS_CLASS(flags_class));
 
-  flags_value = g_flags_get_first_value(flags_class, PYGLIB_PyLong_AS_LONG(self));
+  flags_value = g_flags_get_first_value(flags_class, PYGLIB_PyLong_AsUnsignedLong(self));
   if (flags_value)
       retval = PYGLIB_PyUnicode_FromString(flags_value->value_nick);
   else {
@@ -402,7 +414,7 @@ pyg_flags_get_value_names(PyGFlags *self, void *closure)
 
   retval = PyList_New(0);
   for (i = 0; i < flags_class->n_values; i++)
-      if ((PYGLIB_PyLong_AS_LONG(self) & flags_class->values[i].value) == flags_class->values[i].value)
+      if ((PYGLIB_PyLong_AsUnsignedLong(self) & flags_class->values[i].value) == 
flags_class->values[i].value)
          PyList_Append(retval, PYGLIB_PyUnicode_FromString(flags_class->values[i].value_name));
 
   g_type_class_unref(flags_class);
@@ -422,7 +434,7 @@ pyg_flags_get_value_nicks(PyGFlags *self, void *closure)
 
   retval = PyList_New(0);
   for (i = 0; i < flags_class->n_values; i++)
-      if ((PYGLIB_PyLong_AS_LONG(self) & flags_class->values[i].value) == flags_class->values[i].value)
+      if ((PYGLIB_PyLong_AsUnsignedLong(self) & flags_class->values[i].value) == 
flags_class->values[i].value)
          PyList_Append(retval, PYGLIB_PyUnicode_FromString(flags_class->values[i].value_nick));
 
   g_type_class_unref(flags_class);
diff --git a/gi/_gobject/pygobject-private.h b/gi/_gobject/pygobject-private.h
index 0d1aca0..be2b6e3 100644
--- a/gi/_gobject/pygobject-private.h
+++ b/gi/_gobject/pygobject-private.h
@@ -107,7 +107,7 @@ GType     pyg_type_from_object_strict (PyObject *obj, gboolean strict);
 GType     pyg_type_from_object (PyObject *obj);
 
 gint pyg_enum_get_value  (GType enum_type, PyObject *obj, gint *val);
-gint pyg_flags_get_value (GType flag_type, PyObject *obj, gint *val);
+gint pyg_flags_get_value (GType flag_type, PyObject *obj, guint *val);
 int pyg_pyobj_to_unichar_conv (PyObject* py_obj, void* ptr);
 
 typedef PyObject *(* fromvaluefunc)(const GValue *value);
@@ -181,7 +181,8 @@ const gchar * pyg_constant_strip_prefix(const gchar *name, const gchar *strip_pr
 
 /* pygflags */
 typedef struct {
-       PYGLIB_PyLongObject parent;
+    PYGLIB_PyLongObject parent;
+    int zero_pad; /* must always be 0 */
     GType gtype;
 } PyGFlags;
 
@@ -194,13 +195,14 @@ extern PyObject * pyg_flags_add        (PyObject *   module,
                                        const char * strip_prefix,
                                        GType        gtype);
 extern PyObject * pyg_flags_from_gtype (GType        gtype,
-                                       int          value);
+                                       guint        value);
 
 /* pygenum */
 #define PyGEnum_Check(x) (PyObject_IsInstance((PyObject *)x, (PyObject *)&PyGEnum_Type) && 
g_type_is_a(((PyGFlags*)x)->gtype, G_TYPE_ENUM))
 
 typedef struct {
-       PYGLIB_PyLongObject parent;
+    PYGLIB_PyLongObject parent;
+    int zero_pad; /* must always be 0 */
     GType gtype;
 } PyGEnum;
 
diff --git a/gi/_gobject/pygobject.h b/gi/_gobject/pygobject.h
index 92dc030..cfd88d7 100644
--- a/gi/_gobject/pygobject.h
+++ b/gi/_gobject/pygobject.h
@@ -103,7 +103,7 @@ struct _PyGObject_Functions {
     PyObject *(* type_wrapper_new)(GType type);
 
     gint (* enum_get_value)(GType enum_type, PyObject *obj, gint *val);
-    gint (* flags_get_value)(GType flag_type, PyObject *obj, gint *val);
+    gint (* flags_get_value)(GType flag_type, PyObject *obj, guint *val);
     void (* register_gtype_custom)(GType gtype,
                            PyObject *(* from_func)(const GValue *value),
                            int (* to_func)(GValue *value, PyObject *obj));
@@ -168,7 +168,7 @@ struct _PyGObject_Functions {
                           const char *type_name_,
                           const char *strip_prefix,
                           GType gtype);
-    PyObject* (*flags_from_gtype)(GType gtype, int value);
+    PyObject* (*flags_from_gtype)(GType gtype, guint value);
 
     gboolean threads_enabled;
     int       (*enable_threads) (void);
diff --git a/gi/_gobject/pygtype.c b/gi/_gobject/pygtype.c
index 227178f..94014ff 100644
--- a/gi/_gobject/pygtype.c
+++ b/gi/_gobject/pygtype.c
@@ -538,7 +538,7 @@ pyg_enum_get_value(GType enum_type, PyObject *obj, gint *val)
  * Returns: 0 on success or -1 on failure
  */
 gint
-pyg_flags_get_value(GType flag_type, PyObject *obj, gint *val)
+pyg_flags_get_value(GType flag_type, PyObject *obj, guint *val)
 {
     GFlagsClass *fclass = NULL;
     gint res = -1;
@@ -548,7 +548,7 @@ pyg_flags_get_value(GType flag_type, PyObject *obj, gint *val)
        *val = 0;
        res = 0;
     } else if (PYGLIB_PyLong_Check(obj)) {
-       *val = PYGLIB_PyLong_AsLong(obj);
+       *val = PYGLIB_PyLong_AsUnsignedLong(obj);
        res = 0;
     } else if (PyLong_Check(obj)) {
         *val = PyLong_AsLongLong(obj);
@@ -944,7 +944,7 @@ pyg_value_from_pyobject(GValue *value, PyObject *obj)
        break;
     case G_TYPE_FLAGS:
        {
-           gint val = 0;
+           guint val = 0;
            if (pyg_flags_get_value(G_VALUE_TYPE(value), obj, &val) < 0) {
                PyErr_Clear();
                return -1;
diff --git a/tests/test_overrides_gdk.py b/tests/test_overrides_gdk.py
index 070acb8..c1f10d9 100644
--- a/tests/test_overrides_gdk.py
+++ b/tests/test_overrides_gdk.py
@@ -117,3 +117,18 @@ class TestGdk(unittest.TestCase):
 
         self.assertNotEqual(c, None)
         self.assertRaises(ValueError, Gdk.Cursor, 1, 2, 3)
+
+    def test_flags(self):
+        self.assertEqual(Gdk.ModifierType.META_MASK | 0, 0x10000000)
+        self.assertEqual(hex(Gdk.ModifierType.META_MASK), '0x10000000')
+        self.assertEqual(str(Gdk.ModifierType.META_MASK),
+                         '<flags GDK_META_MASK of type GdkModifierType>')
+
+        self.assertEqual(Gdk.ModifierType.RELEASE_MASK | 0, 0x40000000)
+        self.assertEqual(hex(Gdk.ModifierType.RELEASE_MASK), '0x40000000')
+        self.assertEqual(str(Gdk.ModifierType.RELEASE_MASK),
+                         '<flags GDK_RELEASE_MASK of type GdkModifierType>')
+
+        self.assertEqual(Gdk.ModifierType.RELEASE_MASK | Gdk.ModifierType.META_MASK, 0x50000000)
+        self.assertEqual(str(Gdk.ModifierType.RELEASE_MASK | Gdk.ModifierType.META_MASK),
+                         '<flags GDK_META_MASK | GDK_RELEASE_MASK of type GdkModifierType>')
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]