GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-128715: Expose ctypes.CField, with info attributes by encukou · Pull Request #128950 · python/cpython

Conversation

@encukou

@encukou encukou commented Jan 17, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz reacted with heart emoji
All reactions
  • ❤️ 1 reaction
encukou added 7 commits January 17, 2025 16:59
@encukou
Restore max field size to sys.maxsize, as in Python 3.13 & below
9d4be49
@encukou
PyCField: Split out bit/byte sizes/offsets.
09c81a8
@encukou
Expose CField
c397cf4
@encukou
Add generic checks for all the test structs/unions
20ecd84
@encukou
More testing
60e7b32
@encukou
Tests: import CField from ctypes
18334d8
@encukou
Clarify bit_offset
294b1b8
@bedevere-app bedevere-app Bot mentioned this pull request Jan 17, 2025
Untangle ctypes bitfield size and expose field information #128715
Open
@bedevere-app bedevere-app Bot added the awaiting core review label Jan 17, 2025
encukou added 8 commits January 17, 2025 17:35
@encukou
Add a blurb
52114fa
@encukou
Regen
32d41f3
@encukou
Merge in the main branch
f6f596f
@encukou
include <stdbool.h> in the common header
9b0e7eb
@encukou
Explicit casts
d7bd835
@encukou
Remove problematic assert
06458f6
@encukou
Merge in the main branch
bb41481
@encukou
Add a test for the new info, and fix 'name' for nested anonymous structs
91365b0
picnixz
picnixz reviewed Jan 24, 2025
View reviewed changes
Comment thread Lib/ctypes/_layout.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_ctypes/_support.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_ctypes/_support.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

encukou added 6 commits January 24, 2025 18:30
@encukou
Use subTest
b6d0510
@encukou
Use PyUnicode_FromObject to get an exact PyUnicode
6e279e2
@encukou
Normalize exception message
176de87
@encukou
Fix refcounting
085720e
@encukou
Add pretty spaces
05c9591
@encukou
Merge in the main branch
b77074c
@encukou
encukou marked this pull request as draft January 31, 2025 16:47
@bedevere-app bedevere-app Bot removed the awaiting core review label Jan 31, 2025
encukou added 4 commits January 31, 2025 17:48
@encukou
Fix bit-packed size test for big-endian machines
4e95755
@encukou
Remove size from _layout.py
9cde20e
@encukou
Fix bit_offset for big-endian structs (where bitfields are laid out f…
34865e8
…rom the big end)
@encukou
Merge in the main branch
89fc44d
@encukou
Update documentation for tp_basicsize & tp_itemsize
d9d593c
- 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

picnixz commented Feb 8, 2025

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

encukou added 2 commits February 21, 2025 15:48
@encukou
Merge in the main branch
d52b8c9
@encukou
Skip "is in" test for bitfields of underaligned types (bug filed)
2cdcb5b
@encukou

encukou commented Feb 21, 2025

Copy link
Copy Markdown
Member Author

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!)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou
encukou marked this pull request as ready for review February 25, 2025 12:39
@bedevere-app bedevere-app Bot added the awaiting core review label Feb 25, 2025
@encukou

encukou commented Mar 10, 2025

Copy link
Copy Markdown
Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Mar 10, 2025

Copy link
Copy Markdown
Member

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!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz
picnixz reviewed Mar 10, 2025
View reviewed changes

@picnixz picnixz left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/test/test_ctypes/_support.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_ctypes/test_aligned_structures.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

encukou added 2 commits March 14, 2025 17:18
@encukou
Merge in the main branch
753090a
@encukou
Address review
fde4204
encukou
encukou commented Mar 14, 2025
View reviewed changes

@encukou encukou left a comment

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/test/test_ctypes/test_aligned_structures.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@encukou
One more alignment
b2fddd8
sergey-miryanov
sergey-miryanov reviewed Mar 16, 2025
View reviewed changes
Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

sergey-miryanov
sergey-miryanov reviewed Mar 16, 2025
View reviewed changes
Comment thread Modules/_ctypes/cfield.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@encukou
Don't use self while it's NULL
5ce595c
@encukou

encukou commented Mar 20, 2025

Copy link
Copy Markdown
Member Author

Thanks for the reviews everyone!
I'll merge tomorrow if there are no objections.

sergey-miryanov reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz
picnixz approved these changes Mar 23, 2025
View reviewed changes

@picnixz picnixz left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I've much more to say. My main concern was the unicode-exactness of the name which is now resolved so I'm fine.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Mar 23, 2025
@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 24, 2025
@bedevere-bot

bedevere-bot commented Mar 24, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @encukou for commit 5ce595c 🤖

Results will be shown at:

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

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 24, 2025
@encukou
encukou merged commit 0e53038 into python:main Mar 24, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Mar 24, 2025
@encukou
encukou deleted the ctypes-bitfield-3 branch March 24, 2025 13:18
@encukou

encukou commented Mar 24, 2025

Copy link
Copy Markdown
Member Author

Almost tomorrow.... :)

Thank you for the reviews, everyone!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 1, 2025
@encukou @diegorusso
pythongh-128715: Expose ctypes.CField, with info attributes (pythonGH…
cb6990c
…-128950)

- Restore max field size to sys.maxsize, as in Python 3.13 & below
- PyCField: Split out bit/byte sizes/offsets.
- Expose CField's size/offset data to Python code
- Add generic checks for all the test structs/unions, using the newly exposed attrs
@BHUVANSH855 BHUVANSH855 mentioned this pull request Oct 20, 2026
ctypes.Structure: bitfield of underaligned type can cause read of unrelated memory #130410
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sergey-miryanov sergey-miryanov sergey-miryanov left review comments
@picnixz picnixz picnixz approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@encukou @bedevere-bot @picnixz @sergey-miryanov
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.