[pygobject] Deprecate void pointer fields as general PyObject storage.

[Simon Feltman <sfeltman src gnome org>]

commit 326218a20681c1f2234a6eea1ed800382be57626
Author: Simon Feltman <s feltman gmail com>
Date:   Wed Sep 19 15:37:14 2012 -0700

    Deprecate void pointer fields as general PyObject storage.
    
    Complete deprecation of gpointer struct fields as general
    PyObject storage. Only int types are now allowed.
    Assignment of anything other than an int or None raises
    a TypeError stating the error and  associated bug URL.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=683599

 gi/pygi-argument.c          |   25 ++++++++++++++++----
 gi/pygi-info.c              |   28 +++-------------------
 tests/test_everything.py    |   53 ++++++++++++++++++++++++------------------
 tests/test_overrides_gtk.py |   41 ---------------------------------
 4 files changed, 54 insertions(+), 93 deletions(-)
---
diff --git a/gi/pygi-argument.c b/gi/pygi-argument.c
index bffe9c4..8c6f75d 100644
--- a/gi/pygi-argument.c
+++ b/gi/pygi-argument.c
@@ -879,7 +879,15 @@ _pygi_argument_from_object (PyObject   *object,
     switch (type_tag) {
         case GI_TYPE_TAG_VOID:
             g_warn_if_fail (transfer == GI_TRANSFER_NOTHING);
-            arg.v_pointer = object == Py_None ? NULL : object;
+            if (object == Py_None) {
+                arg.v_pointer = NULL;
+            } else if (!PYGLIB_PyLong_Check(object)  && !PyLong_Check(object)) {
+                PyErr_SetString(PyExc_TypeError,
+                    "Pointer assignment is restricted to integer values. "
+                    "See: https://bugzilla.gnome.org/show_bug.cgi?id=683599";);
+            } else {
+                arg.v_pointer = PyLong_AsVoidPtr (object);
+            }
             break;
         case GI_TYPE_TAG_BOOLEAN:
         {
@@ -1531,15 +1539,22 @@ _pygi_argument_to_object (GIArgument  *arg,
     type_tag = g_type_info_get_tag (type_info);
     switch (type_tag) {
         case GI_TYPE_TAG_VOID:
+        {
             if (g_type_info_is_pointer (type_info) &&
                     (arg->v_pointer != NULL)) {
-                /* Raw Python objects are passed to void* args */
                 g_warn_if_fail (transfer == GI_TRANSFER_NOTHING);
-                object = arg->v_pointer;
-            } else
+                object = PyLong_FromVoidPtr (arg->v_pointer);
+            } else {
+                /* None is used instead of zero for parity with ctypes.
+                 * This is helpful in case the values are being used for
+                 * actual memory addressing, in which case None will
+                 * raise as opposed to 0 which will crash.
+                 */
                 object = Py_None;
-            Py_XINCREF (object);
+                Py_INCREF (object);
+            }
             break;
+        }
         case GI_TYPE_TAG_BOOLEAN:
         {
             object = PyBool_FromLong (arg->v_boolean);
diff --git a/gi/pygi-info.c b/gi/pygi-info.c
index 6f93eee..dc99a83 100644
--- a/gi/pygi-info.c
+++ b/gi/pygi-info.c
@@ -1433,34 +1433,14 @@ _wrap_g_field_info_set_value (PyGIBaseInfo *self,
         g_base_info_unref (info);
     } else if (g_type_info_is_pointer (field_type_info)
             && g_type_info_get_tag (field_type_info) == GI_TYPE_TAG_VOID) {
-        int offset;
 
-        if (py_value != Py_None && !PYGLIB_PyLong_Check(py_value)) {
-            if (PyErr_WarnEx(PyExc_RuntimeWarning,
-                         "Usage of gpointers to store objects is being deprecated. "
-                         "Please use integer values only, see: https://bugzilla.gnome.org/show_bug.cgi?id=683599";,
-                         1))
-                goto out;
-        }
-
-        offset = g_field_info_get_offset ((GIFieldInfo *) self->info);
         value = _pygi_argument_from_object (py_value, field_type_info, GI_TRANSFER_NOTHING);
+        if (PyErr_Occurred()) {
+            goto out;
+        }
 
-        /* Decrement the previous python object stashed on the void pointer.
-         * This seems somewhat dangerous as the code is blindly assuming any
-         * void pointer field stores a python object pointer and then decrefs it.
-         * This is essentially the same as something like:
-         *  Py_XDECREF(struct->void_ptr); */
-        Py_XDECREF(G_STRUCT_MEMBER (gpointer, pointer, offset));
-
-        /* Assign and increment the newly assigned object. At this point the value
-         * arg will hold a pointer the python object "py_value" or NULL.
-         * This is essentially:
-         *  struct->void_ptr = value.v_pointer;
-         *  Py_XINCREF(struct->void_ptr);
-         */
+        int offset = g_field_info_get_offset ((GIFieldInfo *) self->info);
         G_STRUCT_MEMBER (gpointer, pointer, offset) = (gpointer)value.v_pointer;
-        Py_XINCREF(G_STRUCT_MEMBER (gpointer, pointer, offset));
 
         retval = Py_None;
         goto out;
diff --git a/tests/test_everything.py b/tests/test_everything.py
index 4c4535f..3dc4ed8 100644
--- a/tests/test_everything.py
+++ b/tests/test_everything.py
@@ -4,14 +4,10 @@
 
 import unittest
 import traceback
+import ctypes
 import warnings
-import gc
-gc
-
 import sys
-from sys import getrefcount
 
-import copy
 try:
     import cairo
     has_cairo = True
@@ -37,6 +33,17 @@ else:
     UNICHAR = "â"
 
 
+class RawGList(ctypes.Structure):
+    _fields_ = [('data', ctypes.c_void_p),
+                ('next', ctypes.c_void_p),
+                ('prev', ctypes.c_void_p)]
+
+    @classmethod
+    def from_wrapped(cls, obj):
+        offset = sys.getsizeof(object())  # size of PyObject_HEAD
+        return ctypes.POINTER(cls).from_address(id(obj) + offset)
+
+
 @unittest.skipUnless(has_cairo, 'built without cairo support')
 class TestEverything(unittest.TestCase):
 
@@ -212,24 +219,24 @@ class TestEverything(unittest.TestCase):
         data = None
 
     def test_struct_gpointer(self):
-        l1 = GLib.List()
-        self.assertEqual(l1.data, None)
-        init_refcount = getrefcount(l1)
-
-        l1.data = 'foo'
-        self.assertEqual(l1.data, 'foo')
-
-        l2 = l1
-        self.assertEqual(l1.data, l2.data)
-        self.assertEqual(getrefcount(l1), init_refcount + 1)
-
-        l3 = copy.copy(l1)
-        l3.data = 'bar'
-        self.assertEqual(l1.data, 'foo')
-        self.assertEqual(l2.data, 'foo')
-        self.assertEqual(l3.data, 'bar')
-        self.assertEqual(getrefcount(l1), init_refcount + 1)
-        self.assertEqual(getrefcount(l3), init_refcount)
+        glist = GLib.List()
+        raw = RawGList.from_wrapped(glist)
+
+        self.assertEqual(glist.data, None)
+        self.assertEqual(raw.contents.data, None)
+
+        glist.data = 123
+        self.assertEqual(glist.data, 123)
+        self.assertEqual(raw.contents.data, 123)
+
+        glist.data = None
+        self.assertEqual(glist.data, None)
+        self.assertEqual(raw.contents.data, None)
+
+        # Setting to anything other than an int should raise
+        self.assertRaises(TypeError, setattr, glist.data, 'nan')
+        self.assertRaises(TypeError, setattr, glist.data, object())
+        self.assertRaises(TypeError, setattr, glist.data, 123.321)
 
     def test_struct_opaque(self):
         # we should get a sensible error message
diff --git a/tests/test_overrides_gtk.py b/tests/test_overrides_gtk.py
index ee41457..c39f793 100644
--- a/tests/test_overrides_gtk.py
+++ b/tests/test_overrides_gtk.py
@@ -2,8 +2,6 @@
 # vim: tabstop=4 shiftwidth=4 expandtab
 
 import unittest
-import ctypes
-import sys
 
 from compathelper import _unicode, _bytes
 
@@ -18,19 +16,6 @@ except ImportError:
     Gtk = None
 
 
-class RawTreeIter(ctypes.Structure):
-    """Class used for testing Gtk.TreeIter raw data."""
-    _fields_ = [('stamp', ctypes.c_int),
-                ('user_data', ctypes.c_void_p),
-                ('user_data2', ctypes.c_void_p),
-                ('user_data3', ctypes.c_void_p)]
-
-    @classmethod
-    def from_iter(cls, iter):
-        offset = sys.getsizeof(object())  # size of PyObject_HEAD
-        return ctypes.POINTER(cls).from_address(id(iter) + offset)
-
-
 @unittest.skipUnless(Gtk, 'Gtk not available')
 class TestGtk(unittest.TestCase):
     def test_container(self):
@@ -1432,32 +1417,6 @@ class TestTreeView(unittest.TestCase):
         self.assertEqual(m, store)
         self.assertEqual(store.get_path(s), firstpath)
 
-    def test_tree_iter_user_data_int(self):
-        pyiter = Gtk.TreeIter()
-        rawiter = RawTreeIter.from_iter(pyiter)
-
-        initial_ref_count = sys.getrefcount(1)
-        pyiter.user_data = 1
-
-        # verify setting int value increases refcount of the "1" object
-        self.assertEqual(sys.getrefcount(1), initial_ref_count + 1)
-        # verify the address of the '1' object is what user_data is actually set to.
-        self.assertEqual(id(1), rawiter.contents.user_data)
-
-    def test_tree_iter_user_data_null(self):
-        pyiter = Gtk.TreeIter()
-        rawiter = RawTreeIter.from_iter(pyiter)
-
-        self.assertEqual(pyiter.user_data, None)
-        self.assertEqual(rawiter.contents.user_data, None)
-
-        # Setting user_data to None should not increase None's ref count.
-        # and the raw iters user_data should also come back as None/NULL.
-        initial_ref_count = sys.getrefcount(None)
-        pyiter.user_data = None
-        self.assertEqual(sys.getrefcount(None), initial_ref_count)
-        self.assertEqual(rawiter.contents.user_data, None)
-
 
 @unittest.skipUnless(Gtk, 'Gtk not available')
 class TestTextBuffer(unittest.TestCase):