Skip to content

Commit ac4e5bb

Browse files
committed
bpo-32780: Fix the PEP3118 format string for ctypes.Structure
Without this fix, it would never include padding bytes - an assumption that was only valid in the case when `_pack_` was set - and this case was explicitly not implemented. This should allow conversion from ctypes structs to numpy structs.
1 parent c1e46e9 commit ac4e5bb

File tree

3 files changed

+127
-30
lines changed

3 files changed

+127
-30
lines changed

Lib/ctypes/test/test_pep3118.py

+20-3
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,20 @@ class PackedPoint(Structure):
8686
_pack_ = 2
8787
_fields_ = [("x", c_long), ("y", c_long)]
8888

89+
class PointMidPad(Structure):
90+
_fields_ = [("x", c_byte), ("y", c_uint64)]
91+
92+
class PackedPointMidPad(Structure):
93+
_pack_ = 2
94+
_fields_ = [("x", c_byte), ("y", c_uint64)]
95+
96+
class PointEndPad(Structure):
97+
_fields_ = [("x", c_uint64), ("y", c_byte)]
98+
99+
class PackedPointEndPad(Structure):
100+
_pack_ = 2
101+
_fields_ = [("x", c_uint64), ("y", c_byte)]
102+
89103
class Point2(Structure):
90104
pass
91105
Point2._fields_ = [("x", c_long), ("y", c_long)]
@@ -183,10 +197,13 @@ class Complete(Structure):
183197

184198
## structures and unions
185199

186-
(Point, "T{<l:x:<l:y:}".replace('l', s_long), (), Point),
187-
# packed structures do not implement the pep
188-
(PackedPoint, "B", (), PackedPoint),
189200
(Point2, "T{<l:x:<l:y:}".replace('l', s_long), (), Point2),
201+
(Point, "T{<l:x:<l:y:}".replace('l', s_long), (), Point),
202+
(PackedPoint, "T{<l:x:<l:y:}".replace('l', s_long), (), PackedPoint),
203+
(PointMidPad, "T{<b:x:7x<Q:y:}", (), PointMidPad),
204+
(PackedPointMidPad, "T{<b:x:x<Q:y:}", (), PackedPointMidPad),
205+
(PointEndPad, "T{<Q:x:<b:y:7x}", (), PointEndPad),
206+
(PackedPointEndPad, "T{<Q:x:<b:y:x}", (), PackedPointEndPad),
190207
(EmptyStruct, "T{}", (), EmptyStruct),
191208
# the pep doesn't support unions
192209
(aUnion, "B", (), aUnion),
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Inter-field padding is now inserted into the PEP3118 format strings obtained
2+
from ``ctypes.Structure`` objects, reflecting their true representation in
3+
memory.

Modules/_ctypes/stgdict.c

+104-27
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,48 @@ MakeAnonFields(PyObject *type)
328328
return 0;
329329
}
330330

331+
/*
332+
Compute ceil(log10(x)), for the purpose of determining string lengths.
333+
*/
334+
static Py_ssize_t
335+
clog10(Py_ssize_t n)
336+
{
337+
Py_ssize_t log_n = 0;
338+
while (n > 0) {
339+
log_n++;
340+
n /= 10;
341+
}
342+
return log_n;
343+
}
344+
345+
/*
346+
Append {padding}x to the PEP3118 format string.
347+
*/
348+
char *
349+
_ctypes_alloc_format_padding(const char *prefix, Py_ssize_t padding)
350+
{
351+
char *result;
352+
char *buf;
353+
354+
assert(padding > 0);
355+
356+
if (padding == 1) {
357+
/* Use x instead of 1x, for brevity */
358+
return _ctypes_alloc_format_string(prefix, "x");
359+
}
360+
361+
/* decimal characters + x + null */
362+
buf = PyMem_Malloc(clog10(padding) + 2);
363+
if (buf == NULL) {
364+
PyErr_NoMemory();
365+
return NULL;
366+
}
367+
sprintf(buf, "%zdx", padding);
368+
result = _ctypes_alloc_format_string(prefix, buf);
369+
PyMem_Free(buf);
370+
return result;
371+
}
372+
331373
/*
332374
Retrieve the (optional) _pack_ attribute from a type, the _fields_ attribute,
333375
and create an StgDictObject. Used for Structure and Union subclasses.
@@ -337,7 +379,7 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
337379
{
338380
StgDictObject *stgdict, *basedict;
339381
Py_ssize_t len, offset, size, align, i;
340-
Py_ssize_t union_size, total_align;
382+
Py_ssize_t union_size, total_align, aligned_size;
341383
Py_ssize_t field_size = 0;
342384
int bitofs;
343385
PyObject *isPacked;
@@ -443,12 +485,10 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
443485
}
444486

445487
assert(stgdict->format == NULL);
446-
if (isStruct && !isPacked) {
488+
if (isStruct) {
447489
stgdict->format = _ctypes_alloc_format_string(NULL, "T{");
448490
} else {
449-
/* PEP3118 doesn't support union, or packed structures (well,
450-
only standard packing, but we don't support the pep for
451-
that). Use 'B' for bytes. */
491+
/* PEP3118 doesn't support union. Use 'B' for bytes. */
452492
stgdict->format = _ctypes_alloc_format_string(NULL, "B");
453493
}
454494
if (stgdict->format == NULL)
@@ -515,24 +555,51 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
515555
} else
516556
bitsize = 0;
517557

518-
if (isStruct && !isPacked) {
558+
if (isStruct) {
519559
const char *fieldfmt = dict->format ? dict->format : "B";
520560
const char *fieldname = PyUnicode_AsUTF8(name);
521561
char *ptr;
522562
Py_ssize_t len;
523563
char *buf;
564+
Py_ssize_t last_size = size;
565+
Py_ssize_t padding;
524566

525567
if (fieldname == NULL)
526568
{
527569
Py_DECREF(pair);
528570
return -1;
529571
}
530572

573+
prop = PyCField_FromDesc(desc, i,
574+
&field_size, bitsize, &bitofs,
575+
&size, &offset, &align,
576+
pack, big_endian);
577+
if (prop == NULL) {
578+
Py_DECREF(pair);
579+
return -1;
580+
}
581+
582+
/* number of bytes between the end of the last field and the start
583+
of this one */
584+
padding = ((CFieldObject *)prop)->offset - last_size;
585+
586+
if (padding > 0) {
587+
ptr = stgdict->format;
588+
stgdict->format = _ctypes_alloc_format_padding(ptr, padding);
589+
PyMem_Free(ptr);
590+
if (stgdict->format == NULL) {
591+
Py_DECREF(pair);
592+
Py_DECREF(prop);
593+
return -1;
594+
}
595+
}
596+
531597
len = strlen(fieldname) + strlen(fieldfmt);
532598

533599
buf = PyMem_Malloc(len + 2 + 1);
534600
if (buf == NULL) {
535601
Py_DECREF(pair);
602+
Py_DECREF(prop);
536603
PyErr_NoMemory();
537604
return -1;
538605
}
@@ -550,15 +617,9 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
550617

551618
if (stgdict->format == NULL) {
552619
Py_DECREF(pair);
620+
Py_DECREF(prop);
553621
return -1;
554622
}
555-
}
556-
557-
if (isStruct) {
558-
prop = PyCField_FromDesc(desc, i,
559-
&field_size, bitsize, &bitofs,
560-
&size, &offset, &align,
561-
pack, big_endian);
562623
} else /* union */ {
563624
size = 0;
564625
offset = 0;
@@ -567,14 +628,14 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
567628
&field_size, bitsize, &bitofs,
568629
&size, &offset, &align,
569630
pack, big_endian);
631+
if (prop == NULL) {
632+
Py_DECREF(pair);
633+
return -1;
634+
}
570635
union_size = max(size, union_size);
571636
}
572637
total_align = max(align, total_align);
573638

574-
if (!prop) {
575-
Py_DECREF(pair);
576-
return -1;
577-
}
578639
if (-1 == PyObject_SetAttr(type, name, prop)) {
579640
Py_DECREF(prop);
580641
Py_DECREF(pair);
@@ -585,29 +646,45 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
585646
}
586647
#undef realdict
587648

588-
if (isStruct && !isPacked) {
589-
char *ptr = stgdict->format;
649+
if (!isStruct) {
650+
size = union_size;
651+
}
652+
653+
/* Adjust the size according to the alignment requirements */
654+
aligned_size = ((size + total_align - 1) / total_align) * total_align;
655+
656+
if (isStruct) {
657+
char *ptr;
658+
Py_ssize_t padding;
659+
660+
/* Pad up to the full size of the struct */
661+
padding = aligned_size - size;
662+
if (padding > 0) {
663+
ptr = stgdict->format;
664+
stgdict->format = _ctypes_alloc_format_padding(ptr, padding);
665+
PyMem_Free(ptr);
666+
if (stgdict->format == NULL) {
667+
return -1;
668+
}
669+
}
670+
671+
ptr = stgdict->format;
590672
stgdict->format = _ctypes_alloc_format_string(stgdict->format, "}");
591673
PyMem_Free(ptr);
592674
if (stgdict->format == NULL)
593675
return -1;
594676
}
595677

596-
if (!isStruct)
597-
size = union_size;
598-
599-
/* Adjust the size according to the alignment requirements */
600-
size = ((size + total_align - 1) / total_align) * total_align;
601-
602678
stgdict->ffi_type_pointer.alignment = Py_SAFE_DOWNCAST(total_align,
603679
Py_ssize_t,
604680
unsigned short);
605-
stgdict->ffi_type_pointer.size = size;
681+
stgdict->ffi_type_pointer.size = aligned_size;
606682

607-
stgdict->size = size;
683+
stgdict->size = aligned_size;
608684
stgdict->align = total_align;
609685
stgdict->length = len; /* ADD ffi_ofs? */
610686

687+
611688
/* We did check that this flag was NOT set above, it must not
612689
have been set until now. */
613690
if (stgdict->flags & DICTFLAG_FINAL) {

0 commit comments

Comments
 (0)