-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
bpo-29753: fix merging packed bitfields in ctypes struct/union #19850
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2b159a0
80ee351
192e4b9
de3df46
9f015f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
In ctypes, now packed bitfields are calculated properly and the first item of packed bitfields is now shrank correctly. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,6 +71,18 @@ PyCField_FromDesc(PyObject *desc, Py_ssize_t index, | |
Py_DECREF(self); | ||
return NULL; | ||
} | ||
|
||
#ifndef MS_WIN32 | ||
/* if we have a packed bitfield, calculate the minimum number of bytes we | ||
need to fit it. otherwise use the specified size. */ | ||
if (pack && bitsize) { | ||
size = (bitsize - 1) / 8 + 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (copying the old comment, so that it shows up in the github editor for reviewers) If we have a packed bitfield, calculate the minimum number of bytes needed to fit the bitfield. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd follow this rule: if a comment is necessary/helpful/crucial for reviewers it should be in the source code so that everyone looking at this code at any point later on sees it, not in GitHub comments. |
||
} else | ||
#endif | ||
size = dict->size; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Otherwise, just use the field size. |
||
|
||
proto = desc; | ||
|
||
if (bitsize /* this is a bitfield request */ | ||
&& *pfield_size /* we have a bitfield open */ | ||
#ifdef MS_WIN32 | ||
|
@@ -87,25 +99,26 @@ PyCField_FromDesc(PyObject *desc, Py_ssize_t index, | |
} else if (bitsize /* this is a bitfield request */ | ||
&& *pfield_size /* we have a bitfield open */ | ||
&& dict->size * 8 >= *pfield_size | ||
&& (*pbitofs + bitsize) <= dict->size * 8) { | ||
/* if this is a packed bitfield, always expand it. | ||
otherwise calculate if we need to expand it. */ | ||
&& (((*pbitofs + bitsize) <= dict->size * 8) || pack)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When we have a packed bitfield and it doesn't fit the current open bitfield, always expand it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm quite convinced this comment should go in the source code. |
||
/* expand bit field */ | ||
fieldtype = EXPAND_BITFIELD; | ||
#endif | ||
} else if (bitsize) { | ||
/* start new bitfield */ | ||
fieldtype = NEW_BITFIELD; | ||
*pbitofs = 0; | ||
*pfield_size = dict->size * 8; | ||
/* use our calculated size (size) instead of type size (dict->size), | ||
which can be different for packed bitfields */ | ||
*pfield_size = size * 8; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Start a new bitfield with the size we calculated above. This means if it is a packed bitfield we start a new bitfield with only the needed size, not the specified field size. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto here. |
||
} else { | ||
/* not a bit field */ | ||
fieldtype = NO_BITFIELD; | ||
*pbitofs = 0; | ||
*pfield_size = 0; | ||
} | ||
|
||
size = dict->size; | ||
proto = desc; | ||
|
||
/* Field descriptors for 'c_char * n' are be scpecial cased to | ||
return a Python string instead of an Array object instance... | ||
*/ | ||
|
@@ -170,10 +183,16 @@ PyCField_FromDesc(PyObject *desc, Py_ssize_t index, | |
break; | ||
|
||
case EXPAND_BITFIELD: | ||
*poffset += dict->size - *pfield_size/8; | ||
*psize += dict->size - *pfield_size/8; | ||
/* increase the size if it is a packed bitfield. | ||
EXPAND_BITFIELD should not be selected for non-packed fields if the | ||
current size isn't already enough. */ | ||
if (pack) | ||
FFY00 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
size = (*pbitofs + bitsize - 1) / 8 + 1; | ||
|
||
*poffset += size - *pfield_size/8; | ||
*psize += size - *pfield_size/8; | ||
|
||
*pfield_size = dict->size * 8; | ||
*pfield_size = size * 8; | ||
|
||
if (big_endian) | ||
self->size = (bitsize << 16) + *pfield_size - *pbitofs - bitsize; | ||
|
Uh oh!
There was an error while loading. Please reload this page.