Armin Rigo avatar Armin Rigo committed 2540f3a

Fix: new("struct foo") returns a pointer, but both that pointer 'p'
and the structure 'p[0]' keep alive the structure data.

Comments (0)

Files changed (6)

pypy/module/_cffi_backend/ccallback.py

 from pypy.rlib.objectmodel import compute_unique_id, keepalive_until_here
 from pypy.rlib import clibffi, rweakref, rgc
 
-from pypy.module._cffi_backend.cdataobj import W_CData, W_CDataOwn
+from pypy.module._cffi_backend.cdataobj import W_CData, W_CDataApplevelOwning
 from pypy.module._cffi_backend.ctypefunc import SIZE_OF_FFI_ARG
 
 # ____________________________________________________________
 
 
-class W_CDataCallback(W_CDataOwn):
+class W_CDataCallback(W_CDataApplevelOwning):
     ll_error = lltype.nullptr(rffi.CCHARP.TO)
 
     def __init__(self, space, ctype, w_callable, w_error):

pypy/module/_cffi_backend/cdataobj.py

 
 
 class W_CData(Wrappable):
+    _attrs_ = ['space', '_cdata', 'ctype']
     _immutable_ = True
-    cdata = lltype.nullptr(rffi.CCHARP.TO)
+    _cdata = lltype.nullptr(rffi.CCHARP.TO)
 
     def __init__(self, space, cdata, ctype):
         from pypy.module._cffi_backend import ctypeprim
         space = self.space
         i = space.getindex_w(w_index, space.w_IndexError)
         self.ctype._check_subscript_index(self, i)
-        ctitem = self.ctype.ctitem
-        w_o = ctitem.convert_to_object(
-            rffi.ptradd(self._cdata, i * ctitem.size))
+        w_o = self._do_getitem(i)
         keepalive_until_here(self)
         return w_o
 
+    def _do_getitem(self, i):
+        ctitem = self.ctype.ctitem
+        return ctitem.convert_to_object(
+            rffi.ptradd(self._cdata, i * ctitem.size))
+
     def setitem(self, w_index, w_value):
         space = self.space
         i = space.getindex_w(w_index, space.w_IndexError)
         return length
 
 
-class W_CDataOwnFromCasted(W_CData):
+class W_CDataApplevelOwning(W_CData):
+    """This is the abstract base class for classes that are of the app-level
+    type '_cffi_backend.CDataOwn'.  These are weakrefable."""
+    _attrs_ = []
+
+    def _owning_num_bytes(self):
+        return self.ctype.size
+
+    def repr(self):
+        return self.space.wrap("<cdata '%s' owning %d bytes>" % (
+            self.ctype.name, self._owning_num_bytes()))
+
+
+class W_CDataNewOwning(W_CDataApplevelOwning):
+    """This is the class used for the app-level type
+    '_cffi_backend.CDataOwn' created by newp()."""
+    _attrs_ = []
+
+    def __init__(self, space, size, ctype):
+        cdata = lltype.malloc(rffi.CCHARP.TO, size, flavor='raw', zero=True)
+        W_CDataApplevelOwning.__init__(self, space, cdata, ctype)
+
+    @rgc.must_be_light_finalizer
+    def __del__(self):
+        lltype.free(self._cdata, flavor='raw')
+
+
+class W_CDataNewOwningLength(W_CDataNewOwning):
+    """Subclass with an explicit length, for allocated instances of
+    the C type 'foo[]'."""
+    _attrs_ = ['length']
+
+    def __init__(self, space, size, ctype, length):
+        W_CDataNewOwning.__init__(self, space, size, ctype)
+        self.length = length
+
+    def get_array_length(self):
+        return self.length
+
+
+class W_CDataPtrToStructOrUnion(W_CDataApplevelOwning):
+    """This subclass is used for the pointer returned by new('struct foo').
+    It has a strong reference to a W_CDataNewOwning that really owns the
+    struct, which is the object returned by the app-level expression 'p[0]'."""
+    _attrs_ = ['structobj']
+
+    def __init__(self, space, cdata, ctype, structobj):
+        W_CDataApplevelOwning.__init__(self, space, cdata, ctype)
+        self.structobj = structobj
+
+    def _owning_num_bytes(self):
+        from pypy.module._cffi_backend.ctypeptr import W_CTypePtrBase
+        ctype = self.ctype
+        assert isinstance(ctype, W_CTypePtrBase)
+        return ctype.ctitem.size
+
+    def _do_getitem(self, i):
+        return self.structobj
+
+
+class W_CDataCasted(W_CData):
+    """This subclass is used by the results of cffi.cast('int', x)
+    or other primitive explicitly-casted types.  Relies on malloc'ing
+    small bits of memory (e.g. just an 'int').  Its point is to not be
+    a subclass of W_CDataApplevelOwning."""
+    _attrs_ = []
 
     def __init__(self, space, size, ctype):
         cdata = lltype.malloc(rffi.CCHARP.TO, size, flavor='raw', zero=True)
         lltype.free(self._cdata, flavor='raw')
 
 
-class W_CDataOwn(W_CDataOwnFromCasted):
-
-    def repr(self):
-        return self.space.wrap("<cdata '%s' owning %d bytes>" % (
-            self.ctype.name, self.ctype.size))
-
-
-class W_CDataOwnLength(W_CDataOwn):
-
-    def __init__(self, space, size, ctype, length):
-        W_CDataOwn.__init__(self, space, size, ctype)
-        self.length = length
-
-    def get_array_length(self):
-        return self.length
-
-
 common_methods = dict(
     __nonzero__ = interp2app(W_CData.nonzero),
     __int__ = interp2app(W_CData.int),
     )
 W_CData.typedef.acceptable_as_base_class = False
 
-W_CDataOwn.typedef = TypeDef(
+W_CDataApplevelOwning.typedef = TypeDef(
     '_cffi_backend.CDataOwn',
     __base = W_CData.typedef,
-    __repr__ = interp2app(W_CDataOwn.repr),
-    __weakref__ = make_weakref_descr(W_CDataOwn),
+    __repr__ = interp2app(W_CDataApplevelOwning.repr),
+    __weakref__ = make_weakref_descr(W_CDataApplevelOwning),
     **common_methods
     )
-W_CDataOwn.typedef.acceptable_as_base_class = False
+W_CDataApplevelOwning.typedef.acceptable_as_base_class = False

pypy/module/_cffi_backend/ctypearray.py

                 raise OperationError(space.w_OverflowError,
                     space.wrap("array size would overflow a ssize_t"))
             #
-            cdata = cdataobj.W_CDataOwnLength(space, datasize, self, length)
+            cdata = cdataobj.W_CDataNewOwningLength(space, datasize,
+                                                    self, length)
         #
         else:
-            cdata = cdataobj.W_CDataOwn(space, datasize, self)
+            cdata = cdataobj.W_CDataNewOwning(space, datasize, self)
         #
         if not space.is_w(w_init, space.w_None):
             self.convert_from_object(cdata._cdata, w_init)

pypy/module/_cffi_backend/ctypeprim.py

             value = self.cast_str(w_ob)
         else:
             value = misc.as_unsigned_long_long(space, w_ob, strict=False)
-        w_cdata = cdataobj.W_CDataOwnFromCasted(space, self.size, self)
+        w_cdata = cdataobj.W_CDataCasted(space, self.size, self)
         w_cdata.write_raw_integer_data(value)
         return w_cdata
 
             value = self.cast_str(w_ob)
         else:
             value = space.float_w(w_ob)
-        w_cdata = cdataobj.W_CDataOwnFromCasted(space, self.size, self)
+        w_cdata = cdataobj.W_CDataCasted(space, self.size, self)
         w_cdata.write_raw_float_data(value)
         return w_cdata
 

pypy/module/_cffi_backend/ctypeptr.py

 from pypy.rlib.objectmodel import keepalive_until_here
 
 from pypy.module._cffi_backend.ctypeobj import W_CType
-from pypy.module._cffi_backend.ctypeprim import W_CTypePrimitiveChar
 from pypy.module._cffi_backend import cdataobj, misc
 
 
 
     def __init__(self, space, size, extra, extra_position, ctitem,
                  could_cast_anything=True):
+        from pypy.module._cffi_backend.ctypeprim import W_CTypePrimitiveChar
+        from pypy.module._cffi_backend.ctypestruct import W_CTypeStructOrUnion
         name, name_position = ctitem.insert_name(extra, extra_position)
         W_CType.__init__(self, space, size, name, name_position)
         # this is the "underlying type":
         self.ctitem = ctitem
         self.can_cast_anything = could_cast_anything and ctitem.cast_anything
         self.is_char_ptr_or_array = isinstance(ctitem, W_CTypePrimitiveChar)
+        self.is_struct_ptr = isinstance(ctitem, W_CTypeStructOrUnion)
 
 
 class W_CTypePtrBase(W_CTypePtrOrArray):
         W_CTypePtrBase.__init__(self, space, size, extra, 2, ctitem)
 
     def str(self, cdataobj):
-        if isinstance(self.ctitem, W_CTypePrimitiveChar):
+        if self.is_char_ptr_or_array:
             if not cdataobj._cdata:
                 space = self.space
                 raise operationerrfmt(space.w_RuntimeError,
             raise operationerrfmt(space.w_TypeError,
                 "cannot instantiate ctype '%s' of unknown size",
                                   self.name)
-        if isinstance(ctitem, W_CTypePrimitiveChar):
-            datasize *= 2       # forcefully add a null character
-        cdata = cdataobj.W_CDataOwn(space, datasize, self)
+        if self.is_struct_ptr:
+            # 'newp' on a struct-or-union pointer: in this case, we return
+            # a W_CDataPtrToStruct object which has a strong reference
+            # to a W_CDataNewOwning that really contains the structure.
+            cdatastruct = cdataobj.W_CDataNewOwning(space, datasize, ctitem)
+            cdata = cdataobj.W_CDataPtrToStructOrUnion(space,
+                                                       cdatastruct._cdata,
+                                                       self, cdatastruct)
+        else:
+            if self.is_char_ptr_or_array:
+                datasize *= 2       # forcefully add a null character
+            cdata = cdataobj.W_CDataNewOwning(space, datasize, self)
+        #
         if not space.is_w(w_init, space.w_None):
             ctitem.convert_from_object(cdata._cdata, w_init)
             keepalive_until_here(cdata)
         return cdata
 
     def _check_subscript_index(self, w_cdata, i):
-        if isinstance(w_cdata, cdataobj.W_CDataOwn) and i != 0:
+        if isinstance(w_cdata, cdataobj.W_CDataApplevelOwning) and i != 0:
             space = self.space
             raise operationerrfmt(space.w_IndexError,
                                   "cdata '%s' can only be indexed by 0",

pypy/module/_cffi_backend/test/_backend_test_c.py

     f = callback(BFunc, cb)
     s = f(10)
     assert typeof(s) is BStruct
-    assert repr(s).startswith("<cdata 'struct foo' owning ")
+    assert repr(s) in ["<cdata 'struct foo' owning 12 bytes>",
+                       "<cdata 'struct foo' owning 16 bytes>"]
     assert s.a == -10
     assert s.b == 1E-42
 
     # struct that *also* owns the memory
     BStruct = new_struct_type("foo")
     BStructPtr = new_pointer_type(BStruct)
-    complete_struct_or_union(BStruct, [('a1', new_primitive_type("int"), -1)])
+    complete_struct_or_union(BStruct, [('a1', new_primitive_type("int"), -1),
+                                       ('a2', new_primitive_type("int"), -1),
+                                       ('a3', new_primitive_type("int"), -1)])
     p = newp(BStructPtr)
-    assert repr(p) == "<cdata 'struct foo *' owning 4 bytes>"
+    assert repr(p) == "<cdata 'struct foo *' owning 12 bytes>"
     q = p[0]
-    assert repr(q) == "<cdata 'struct foo' owning 4 bytes>"
+    assert repr(q) == "<cdata 'struct foo' owning 12 bytes>"
     q.a1 = 123456
     assert p.a1 == 123456
     del p
     import gc; gc.collect()
     assert q.a1 == 123456
-    assert repr(q) == "<cdata 'struct foo' owning 4 bytes>"
+    assert repr(q) == "<cdata 'struct foo' owning 12 bytes>"
     assert q.a1 == 123456
 
 def test_nokeepalive_struct():
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.