gh-128715: Expose ctypes.CField, with info attributes #128950

encukou · 2025-01-17T16:34:01Z

Expose the type of struct/union field descriptors (_ctypes._CField) as ctypes.CField. (This is mainly to make it easier to refer to it -- for typing and documentation. Creating CFields manually is not supported.)

Add attributes for easier introspection:

name & type, as defined in _fields_
byte_size & byte_offset, specify the chunk of memory to read/write
bit_size & bit_offset, which specify the shift & mask for bitfields
is_bitfield & is_anonymous booleans

The existing offset remains, as an alias for byte_offset.
The existing size is unchanged. Usually it is the same as byte_size, but for bitfields, it contains a bit-packed combination of bit_size & bit_offset.

Update the repr() of CField.

Use the same values internally as well. (Except bit_size, which might overflow Py_ssize_t for very large types. Instead, in C use bitfield_size, which is 0 for non-bitfields and thus serves as is_bitfield flag. Different name used clarity.)
Old names are removed from the C implementation to ensure a clean transition.
For simplicity, I keep byte_size in CFieldObject for now, even though it's redundant: it's the size of the underlying type.

Add a generic test that ensures the values are consistent, and that we don't have overlapping fields in structs. Use this check throughout test_ctypes, wherever a potentially interesting Structure or Union is created. (Tests for how simple structs behave after they're created don't need the checks, but I erred on the side of adding checks.)

Lift the restriction on maximum field size that was temporarily added in GH-126938. The max size is now Py_ssize_t.

This PR does not yet touch cfield.c getters & setters: the bit-packed “size” argument is computed and passed to those.

Issue: Untangle ctypes bitfield size and expose field information #128715

📚 Documentation preview 📚: https://cpython-previews--128950.org.readthedocs.build/

Lib/ctypes/_layout.py

Lib/test/test_ctypes/_support.py

Modules/_ctypes/cfield.c

…rom the big end)

encukou · 2025-02-07T18:49:37Z

Thank you for the review!
Here's more...

I spent two Fridays learning the details about big-endian bitfield layout. The result is in the new commits here.

The C standards use the term "addressable storage unit" for the memory that bitfields live in. I now use storage unit in the docs as well.
The bit_offset attribute is now defined as the bit shift amount (on a native integer) needed to get the bitfield value from the storage unit. (That's what ctypes used in size before; I hope it'll now be clear to anyone who comes after me.)

bedevere-bot · 2025-02-07T19:08:19Z

🤖 New build scheduled with the buildbot fleet by @encukou for commit 7ce3cb9 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F30617%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

encukou · 2025-02-07T19:24:51Z

I'll continue next week :)

- Add alignment requirement - Mention that ob_size is unreliable if you don't control it - Add some links for context - basicsize should include the base type in generaly not just PyObject This adds a “by-the-way” link to `PyObject_New`, which shouldn't be used for GC types. In order to be comfortable linking to it, I also add a link to `PyObject_GC_New` from its docs. And the same for `*Var` variants, while I'm here.

picnixz · 2025-02-08T13:37:13Z

Should I review what you wrote now or do you prefer me to wait?

encukou · 2025-02-21T15:42:45Z

Sorry, I missed the comment! I'd appreciate a review.

The failure on 32-bit Debian is due to an existing bug, but such an exotic one that it's not worth fixing now: #130410
(It shows that the tests work, though!)

encukou · 2025-03-10T10:45:40Z

@picnixz, should I wait for your review?
No pressure, feel free to say no :)

picnixz · 2025-03-10T10:50:17Z

Oh I forgot... I'll have maybe a bit of time before taking my plane or I'll be back on Wednesday. If you want to wait until then, I'll review it otherwise just go ahead!

picnixz

Some comments (hard to do a better review on a phone)

Lib/test/test_ctypes/_support.py

picnixz · 2025-03-10T14:13:29Z

Lib/test/test_ctypes/test_aligned_structures.py


-
-class TestAlignedStructures(unittest.TestCase):
+class TestAlignedStructures(unittest.TestCase, StructCheckMixin):


I am not entirely sure but shouldn't mixin classes appear before non-mixin ones?

If the methods/attributes don't overlap, it doesn't matter.

picnixz · 2025-03-10T14:16:24Z

Modules/_ctypes/cfield.c

@@ -56,51 +55,41 @@ Py_ssize_t LOW_BIT(Py_ssize_t offset);
 @classmethod
 _ctypes.CField.__new__ as PyCField_new

+    *


Maybe the name could be non-keyword?

Let's do that when it's exposed as a public API.

Modules/_ctypes/cfield.c

picnixz · 2025-03-10T14:26:14Z

Modules/_ctypes/cfield.c

+            (Py_ssize_t)field->bitfield_size,
+            (Py_ssize_t)field->bit_offset);


Is it usually preferrable to have a cast and use zd or directly use PRI8u?

Generally, I don't know. Py_ssize_t seems more readable to me.

Here, it's an intentional (but somewhat obscure) choice -- these are sizes; they're just packed rather aggressively. Before there's a dedicated CBitField subclass, I don't want to waste memory on bitfield data.

encukou

Thank you for the review!

encukou · 2025-03-14T16:20:46Z

Lib/test/test_ctypes/test_aligned_structures.py


-
-class TestAlignedStructures(unittest.TestCase):
+class TestAlignedStructures(unittest.TestCase, StructCheckMixin):


If the methods/attributes don't overlap, it doesn't matter.

encukou · 2025-03-14T16:22:42Z

Modules/_ctypes/cfield.c

@@ -56,51 +55,41 @@ Py_ssize_t LOW_BIT(Py_ssize_t offset);
 @classmethod
 _ctypes.CField.__new__ as PyCField_new

+    *


Let's do that when it's exposed as a public API.

encukou · 2025-03-14T16:23:49Z

Modules/_ctypes/cfield.c

+                        name, byte_size);
+        }
+        bitfield_size = PyLong_AsSsize_t(bit_size_obj);
+        if ((bitfield_size <= 0) || (bitfield_size > 255)) {


Yeah, I don't trust C's operator precedence :)

encukou · 2025-03-14T16:35:28Z

Modules/_ctypes/cfield.c

+            (Py_ssize_t)field->bitfield_size,
+            (Py_ssize_t)field->bit_offset);


Generally, I don't know. Py_ssize_t seems more readable to me.

Here, it's an intentional (but somewhat obscure) choice -- these are sizes; they're just packed rather aggressively. Before there's a dedicated CBitField subclass, I don't want to waste memory on bitfield data.

sergey-miryanov · 2025-03-16T09:45:43Z

Modules/_ctypes/cfield.c

@@ -110,17 +99,12 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto,
                     "type of field %R must be a C type", self->name);


At this point self is NULL. I think should be just name.

We should convert name to an exact Unicode object first though but not bind it to self yet.

D'oh, thanks!

%R on the input is fine for error messages -- the user is getting what they asked for.

sergey-miryanov · 2025-03-16T10:24:58Z

Modules/_ctypes/cfield.c

+    if (!self) {
+        return NULL;
+    }
+    self->name = PyUnicode_FromObject(name);


@picnixz Eah, move this to the top and add a cleanup on error, if I get you right.

encukou added 7 commits January 17, 2025 16:59

Restore max field size to sys.maxsize, as in Python 3.13 & below

9d4be49

PyCField: Split out bit/byte sizes/offsets.

09c81a8

Expose CField

c397cf4

Add generic checks for all the test structs/unions

20ecd84

More testing

60e7b32

Tests: import CField from ctypes

18334d8

Clarify bit_offset

294b1b8

bedevere-app bot mentioned this pull request Jan 17, 2025

Untangle ctypes bitfield size and expose field information #128715

Open

bedevere-app bot added the awaiting core review label Jan 17, 2025

encukou added 8 commits January 17, 2025 17:35

Add a blurb

52114fa

Regen

32d41f3

Merge in the main branch

f6f596f

include <stdbool.h> in the common header

9b0e7eb

Explicit casts

d7bd835

Remove problematic assert

06458f6

Merge in the main branch

bb41481

Add a test for the new info, and fix 'name' for nested anonymous structs

91365b0

picnixz reviewed Jan 24, 2025

View reviewed changes

encukou added 6 commits January 24, 2025 18:30

Use subTest

b6d0510

Use PyUnicode_FromObject to get an exact PyUnicode

6e279e2

Normalize exception message

176de87

Fix refcounting

085720e

Add pretty spaces

05c9591

Merge in the main branch

b77074c

encukou marked this pull request as draft January 31, 2025 16:47

bedevere-app bot removed the awaiting core review label Jan 31, 2025

encukou added 4 commits January 31, 2025 17:48

Fix bit-packed size test for big-endian machines

4e95755

Remove size from _layout.py

9cde20e

Fix bit_offset for big-endian structs (where bitfields are laid out f…

34865e8

…rom the big end)

Merge in the main branch

89fc44d

encukou added 3 commits February 7, 2025 19:05

Name the magic constant

f327ffb

Remove unused variable

14270ad

Remove an unacceptable blank line

7ce3cb9

encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 7, 2025

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 7, 2025

encukou added 2 commits February 21, 2025 15:48

Merge in the main branch

d52b8c9

Skip "is in" test for bitfields of underaligned types (bug filed)

2cdcb5b

encukou marked this pull request as ready for review February 25, 2025 12:39

bedevere-app bot added the awaiting core review label Feb 25, 2025

picnixz reviewed Mar 10, 2025

View reviewed changes

encukou added 2 commits March 14, 2025 17:18

Merge in the main branch

753090a

Address review

fde4204

encukou commented Mar 14, 2025

View reviewed changes

One more alignment

b2fddd8

sergey-miryanov reviewed Mar 16, 2025

View reviewed changes

Don't use self while it's NULL

5ce595c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-128715: Expose ctypes.CField, with info attributes #128950

gh-128715: Expose ctypes.CField, with info attributes #128950

encukou commented Jan 17, 2025 •

edited

Loading

encukou commented Feb 7, 2025

bedevere-bot commented Feb 7, 2025

encukou commented Feb 7, 2025

picnixz commented Feb 8, 2025

encukou commented Feb 21, 2025

encukou commented Mar 10, 2025

picnixz commented Mar 10, 2025

picnixz left a comment

picnixz Mar 10, 2025

encukou Mar 14, 2025

picnixz Mar 10, 2025

encukou Mar 14, 2025

picnixz Mar 10, 2025

encukou Mar 14, 2025

encukou left a comment

encukou Mar 14, 2025

encukou Mar 14, 2025

encukou Mar 14, 2025

encukou Mar 14, 2025

sergey-miryanov Mar 16, 2025 •

edited

Loading

picnixz Mar 16, 2025

encukou Mar 17, 2025

sergey-miryanov Mar 16, 2025



		class TestAlignedStructures(unittest.TestCase):
		class TestAlignedStructures(unittest.TestCase, StructCheckMixin):

		(Py_ssize_t)field->bitfield_size,
		(Py_ssize_t)field->bit_offset);

		@@ -110,17 +99,12 @@ PyCField_new_impl(PyTypeObject type, PyObject name, PyObject *proto,
		"type of field %R must be a C type", self->name);

gh-128715: Expose ctypes.CField, with info attributes #128950

Are you sure you want to change the base?

gh-128715: Expose ctypes.CField, with info attributes #128950

Conversation

encukou commented Jan 17, 2025 • edited Loading

encukou commented Feb 7, 2025

bedevere-bot commented Feb 7, 2025

encukou commented Feb 7, 2025

picnixz commented Feb 8, 2025

encukou commented Feb 21, 2025

encukou commented Mar 10, 2025

picnixz commented Mar 10, 2025

picnixz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

encukou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergey-miryanov Mar 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

encukou commented Jan 17, 2025 •

edited

Loading

sergey-miryanov Mar 16, 2025 •

edited

Loading