From be76e3f26e0b907f711497d006b8b83bff04c036 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 24 Sep 2024 02:40:53 +0200 Subject: [PATCH] gh-100980: ctypes: Test, document, and fix finalizing _fields_ (GH-124292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - If setting `_fields_` fails, e.g. with AttributeError, don't set the attribute in `__dict__` - Document the “finalization” behaviour - Beef up tests: add `getattr`, test Union as well as Structure - Put common functionality in a common function Co-authored-by: Peter Bierma --- Doc/library/ctypes.rst | 21 ++++-- Lib/test/test_ctypes/test_struct_fields.py | 67 ++++++++++--------- ...-09-20-18-23-19.gh-issue-100980.8nVAB6.rst | 3 + Modules/_ctypes/_ctypes.c | 33 +++++---- 4 files changed, 68 insertions(+), 56 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-09-20-18-23-19.gh-issue-100980.8nVAB6.rst diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index 59810183d04..a218304653a 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -2499,6 +2499,8 @@ Structured data types Abstract base class for unions in native byte order. + Unions share common attributes and behavior with structures; + see :class:`Structure` documentation for details. .. class:: BigEndianUnion(*args, **kw) @@ -2558,14 +2560,19 @@ fields, or any other data types containing pointer type fields. ... ] - The :attr:`_fields_` class variable must, however, be defined before the - type is first used (an instance is created, :func:`sizeof` is called on it, - and so on). Later assignments to the :attr:`_fields_` class variable will - raise an AttributeError. + The :attr:`!_fields_` class variable can only be set once. + Later assignments will raise an :exc:`AttributeError`. - It is possible to define sub-subclasses of structure types, they inherit - the fields of the base class plus the :attr:`_fields_` defined in the - sub-subclass, if any. + Additionally, the :attr:`!_fields_` class variable must be defined before + the structure or union type is first used: an instance or subclass is + created, :func:`sizeof` is called on it, and so on. + Later assignments to :attr:`!_fields_` will raise an :exc:`AttributeError`. + If :attr:`!_fields_` has not been set before such use, + the structure or union will have no own fields, as if :attr:`!_fields_` + was empty. + + Sub-subclasses of structure types inherit the fields of the base class + plus the :attr:`_fields_` defined in the sub-subclass, if any. .. attribute:: _pack_ diff --git a/Lib/test/test_ctypes/test_struct_fields.py b/Lib/test/test_ctypes/test_struct_fields.py index 7d7a518c013..b5e165f3bae 100644 --- a/Lib/test/test_ctypes/test_struct_fields.py +++ b/Lib/test/test_ctypes/test_struct_fields.py @@ -4,7 +4,9 @@ from ._support import (CField, Py_TPFLAGS_DISALLOW_INSTANTIATION, Py_TPFLAGS_IMMUTABLETYPE) -class StructFieldsTestCase(unittest.TestCase): +NOTHING = object() + +class FieldsTestBase: # Structure/Union classes must get 'finalized' sooner or # later, when one of these things happen: # @@ -14,42 +16,47 @@ class StructFieldsTestCase(unittest.TestCase): # 4. The type is subclassed # # When they are finalized, assigning _fields_ is no longer allowed. + + def assert_final_fields(self, cls, expected=NOTHING): + self.assertRaises(AttributeError, setattr, cls, "_fields_", []) + self.assertEqual(getattr(cls, "_fields_", NOTHING), expected) + def test_1_A(self): - class X(Structure): + class X(self.cls): pass self.assertEqual(sizeof(X), 0) # not finalized X._fields_ = [] # finalized - self.assertRaises(AttributeError, setattr, X, "_fields_", []) + self.assert_final_fields(X, expected=[]) def test_1_B(self): - class X(Structure): + class X(self.cls): _fields_ = [] # finalized - self.assertRaises(AttributeError, setattr, X, "_fields_", []) + self.assert_final_fields(X, expected=[]) def test_2(self): - class X(Structure): + class X(self.cls): pass X() - self.assertRaises(AttributeError, setattr, X, "_fields_", []) + self.assert_final_fields(X) def test_3(self): - class X(Structure): + class X(self.cls): pass - class Y(Structure): + class Y(self.cls): _fields_ = [("x", X)] # finalizes X - self.assertRaises(AttributeError, setattr, X, "_fields_", []) + self.assert_final_fields(X) def test_4(self): - class X(Structure): + class X(self.cls): pass class Y(X): pass - self.assertRaises(AttributeError, setattr, X, "_fields_", []) + self.assert_final_fields(X) Y._fields_ = [] - self.assertRaises(AttributeError, setattr, X, "_fields_", []) + self.assert_final_fields(X) def test_5(self): - class X(Structure): + class X(self.cls): _fields_ = (("char", c_char * 5),) x = X(b'#' * 5) @@ -59,14 +66,8 @@ class StructFieldsTestCase(unittest.TestCase): def test_6(self): self.assertRaises(TypeError, CField) - def test_cfield_type_flags(self): - self.assertTrue(CField.__flags__ & Py_TPFLAGS_IMMUTABLETYPE) - - def test_cfield_inheritance_hierarchy(self): - self.assertEqual(CField.mro(), [CField, object]) - def test_gh99275(self): - class BrokenStructure(Structure): + class BrokenStructure(self.cls): def __init_subclass__(cls, **kwargs): cls._fields_ = [] # This line will fail, `stginfo` is not ready @@ -77,26 +78,28 @@ class StructFieldsTestCase(unittest.TestCase): # __set__ and __get__ should raise a TypeError in case their self # argument is not a ctype instance. def test___set__(self): - class MyCStruct(Structure): + class MyCStruct(self.cls): _fields_ = (("field", c_int),) self.assertRaises(TypeError, MyCStruct.field.__set__, 'wrong type self', 42) - class MyCUnion(Union): - _fields_ = (("field", c_int),) - self.assertRaises(TypeError, - MyCUnion.field.__set__, 'wrong type self', 42) - def test___get__(self): - class MyCStruct(Structure): + class MyCStruct(self.cls): _fields_ = (("field", c_int),) self.assertRaises(TypeError, MyCStruct.field.__get__, 'wrong type self', 42) - class MyCUnion(Union): - _fields_ = (("field", c_int),) - self.assertRaises(TypeError, - MyCUnion.field.__get__, 'wrong type self', 42) +class StructFieldsTestCase(unittest.TestCase, FieldsTestBase): + cls = Structure + + def test_cfield_type_flags(self): + self.assertTrue(CField.__flags__ & Py_TPFLAGS_IMMUTABLETYPE) + + def test_cfield_inheritance_hierarchy(self): + self.assertEqual(CField.mro(), [CField, object]) + +class UnionFieldsTestCase(unittest.TestCase, FieldsTestBase): + cls = Union if __name__ == "__main__": diff --git a/Misc/NEWS.d/next/Library/2024-09-20-18-23-19.gh-issue-100980.8nVAB6.rst b/Misc/NEWS.d/next/Library/2024-09-20-18-23-19.gh-issue-100980.8nVAB6.rst new file mode 100644 index 00000000000..2279c205cae --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-09-20-18-23-19.gh-issue-100980.8nVAB6.rst @@ -0,0 +1,3 @@ +The :attr:`~ctypes.Structure._fields_` attribute of +:class:`ctypes.Structure` and :class:`~ctypes.Union` is no longer set if +the setattr operation raises an error. diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 1d9e1699022..951e6914ba6 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -1067,32 +1067,31 @@ CType_Type_repeat(PyObject *self, Py_ssize_t length) return PyCArrayType_from_ctype(st, self, length); } +static int +_structunion_setattro(PyObject *self, PyObject *key, PyObject *value, int is_struct) +{ + /* XXX Should we disallow deleting _fields_? */ + if (PyUnicode_Check(key) + && _PyUnicode_EqualToASCIIString(key, "_fields_")) + { + if (PyCStructUnionType_update_stginfo(self, value, is_struct) < 0) { + return -1; + } + } + + return PyType_Type.tp_setattro(self, key, value); +} static int PyCStructType_setattro(PyObject *self, PyObject *key, PyObject *value) { - /* XXX Should we disallow deleting _fields_? */ - if (-1 == PyType_Type.tp_setattro(self, key, value)) - return -1; - - if (value && PyUnicode_Check(key) && - _PyUnicode_EqualToASCIIString(key, "_fields_")) - return PyCStructUnionType_update_stginfo(self, value, 1); - return 0; + return _structunion_setattro(self, key, value, 1); } - static int UnionType_setattro(PyObject *self, PyObject *key, PyObject *value) { - /* XXX Should we disallow deleting _fields_? */ - if (-1 == PyType_Type.tp_setattro(self, key, value)) - return -1; - - if (PyUnicode_Check(key) && - _PyUnicode_EqualToASCIIString(key, "_fields_")) - return PyCStructUnionType_update_stginfo(self, value, 0); - return 0; + return _structunion_setattro(self, key, value, 0); } static PyType_Slot pycstruct_type_slots[] = {