From 25be3f68e02b06c43dfa9eb831fddd69fa33fc64 Mon Sep 17 00:00:00 2001 From: Meador Inge Date: Wed, 18 Jul 2012 23:51:05 -0500 Subject: [PATCH] Issue #6493: Fix handling of c_uint32 bitfields with width of 32 on Windows. --- Lib/ctypes/test/test_bitfields.py | 20 ++++++++++++ Misc/NEWS | 3 ++ Modules/_ctypes/cfield.c | 51 ++++++++++++++----------------- 3 files changed, 46 insertions(+), 28 deletions(-) diff --git a/Lib/ctypes/test/test_bitfields.py b/Lib/ctypes/test/test_bitfields.py index e9c46c2c203..090fb4dfc5e 100644 --- a/Lib/ctypes/test/test_bitfields.py +++ b/Lib/ctypes/test/test_bitfields.py @@ -240,5 +240,25 @@ class BitFieldTest(unittest.TestCase): _anonymous_ = ["_"] _fields_ = [("_", X)] + @unittest.skipUnless(hasattr(ctypes, "c_uint32"), "c_int32 is required") + def test_uint32(self): + class X(Structure): + _fields_ = [("a", c_uint32, 32)] + x = X() + x.a = 10 + self.assertEquals(x.a, 10) + x.a = 0xFDCBA987 + self.assertEquals(x.a, 0xFDCBA987) + + @unittest.skipUnless(hasattr(ctypes, "c_uint64"), "c_int64 is required") + def test_uint64(self): + class X(Structure): + _fields_ = [("a", c_uint64, 64)] + x = X() + x.a = 10 + self.assertEquals(x.a, 10) + x.a = 0xFEDCBA9876543211 + self.assertEquals(x.a, 0xFEDCBA9876543211) + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS b/Misc/NEWS index 6a2abb5d56c..c5fc1bd33e7 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -87,6 +87,9 @@ Core and Builtins Library ------- +- Issue #6493: An issue in ctypes on Windows that caused structure bitfields + of type ctypes.c_uint32 and width 32 to incorrectly be set has been fixed. + - Issue #14635: telnetlib will use poll() rather than select() when possible to avoid failing due to the select() file descriptor limit. diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index 1aab202d685..5e0fc759ff1 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -431,12 +431,8 @@ get_ulonglong(PyObject *v, unsigned PY_LONG_LONG *p) #define LOW_BIT(x) ((x) & 0xFFFF) #define NUM_BITS(x) ((x) >> 16) -/* This seems nore a compiler issue than a Windows/non-Windows one */ -#ifdef MS_WIN32 -# define BIT_MASK(size) ((1 << NUM_BITS(size))-1) -#else -# define BIT_MASK(size) ((1LL << NUM_BITS(size))-1) -#endif +/* Doesn't work if NUM_BITS(size) == 0, but it never happens in SET() call. */ +#define BIT_MASK(type, size) (((((type)1 << (NUM_BITS(size) - 1)) - 1) << 1) + 1) /* This macro CHANGES the first parameter IN PLACE. For proper sign handling, we must first shift left, then right. @@ -448,10 +444,10 @@ get_ulonglong(PyObject *v, unsigned PY_LONG_LONG *p) } /* This macro RETURNS the first parameter with the bit field CHANGED. */ -#define SET(x, v, size) \ +#define SET(type, x, v, size) \ (NUM_BITS(size) ? \ - ( ( x & ~(BIT_MASK(size) << LOW_BIT(size)) ) | ( (v & BIT_MASK(size)) << LOW_BIT(size) ) ) \ - : v) + ( ( (type)x & ~(BIT_MASK(type, size) << LOW_BIT(size)) ) | ( ((type)v & BIT_MASK(type, size)) << LOW_BIT(size) ) ) \ + : (type)v) /* byte swapping macros */ #define SWAP_2(v) \ @@ -523,7 +519,7 @@ b_set(void *ptr, PyObject *value, Py_ssize_t size) long val; if (get_long(value, &val) < 0) return NULL; - *(signed char *)ptr = (signed char)SET(*(signed char *)ptr, (signed char)val, size); + *(signed char *)ptr = SET(signed char, *(signed char *)ptr, val, size); _RET(value); } @@ -542,8 +538,7 @@ B_set(void *ptr, PyObject *value, Py_ssize_t size) unsigned long val; if (get_ulong(value, &val) < 0) return NULL; - *(unsigned char *)ptr = (unsigned char)SET(*(unsigned char*)ptr, - (unsigned short)val, size); + *(unsigned char *)ptr = SET(unsigned char, *(unsigned char*)ptr, val, size); _RET(value); } @@ -564,7 +559,7 @@ h_set(void *ptr, PyObject *value, Py_ssize_t size) if (get_long(value, &val) < 0) return NULL; memcpy(&x, ptr, sizeof(x)); - x = SET(x, (short)val, size); + x = SET(short, x, val, size); memcpy(ptr, &x, sizeof(x)); _RET(value); } @@ -579,7 +574,7 @@ h_set_sw(void *ptr, PyObject *value, Py_ssize_t size) return NULL; memcpy(&field, ptr, sizeof(field)); field = SWAP_2(field); - field = SET(field, (short)val, size); + field = SET(short, field, val, size); field = SWAP_2(field); memcpy(ptr, &field, sizeof(field)); _RET(value); @@ -612,7 +607,7 @@ H_set(void *ptr, PyObject *value, Py_ssize_t size) if (get_ulong(value, &val) < 0) return NULL; memcpy(&x, ptr, sizeof(x)); - x = SET(x, (unsigned short)val, size); + x = SET(unsigned short, x, val, size); memcpy(ptr, &x, sizeof(x)); _RET(value); } @@ -626,7 +621,7 @@ H_set_sw(void *ptr, PyObject *value, Py_ssize_t size) return NULL; memcpy(&field, ptr, sizeof(field)); field = SWAP_2(field); - field = SET(field, (unsigned short)val, size); + field = SET(unsigned short, field, val, size); field = SWAP_2(field); memcpy(ptr, &field, sizeof(field)); _RET(value); @@ -660,7 +655,7 @@ i_set(void *ptr, PyObject *value, Py_ssize_t size) if (get_long(value, &val) < 0) return NULL; memcpy(&x, ptr, sizeof(x)); - x = SET(x, (int)val, size); + x = SET(int, x, val, size); memcpy(ptr, &x, sizeof(x)); _RET(value); } @@ -674,7 +669,7 @@ i_set_sw(void *ptr, PyObject *value, Py_ssize_t size) return NULL; memcpy(&field, ptr, sizeof(field)); field = SWAP_INT(field); - field = SET(field, (int)val, size); + field = SET(int, field, val, size); field = SWAP_INT(field); memcpy(ptr, &field, sizeof(field)); _RET(value); @@ -761,7 +756,7 @@ I_set(void *ptr, PyObject *value, Py_ssize_t size) if (get_ulong(value, &val) < 0) return NULL; memcpy(&x, ptr, sizeof(x)); - x = SET(x, (unsigned int)val, size); + x = SET(unsigned int, x, val, size); memcpy(ptr, &x, sizeof(x)); _RET(value); } @@ -774,7 +769,7 @@ I_set_sw(void *ptr, PyObject *value, Py_ssize_t size) if (get_ulong(value, &val) < 0) return NULL; memcpy(&field, ptr, sizeof(field)); - field = (unsigned int)SET(field, (unsigned int)val, size); + field = SET(unsigned int, field, (unsigned int)val, size); field = SWAP_INT(field); memcpy(ptr, &field, sizeof(field)); _RET(value); @@ -808,7 +803,7 @@ l_set(void *ptr, PyObject *value, Py_ssize_t size) if (get_long(value, &val) < 0) return NULL; memcpy(&x, ptr, sizeof(x)); - x = SET(x, val, size); + x = SET(long, x, val, size); memcpy(ptr, &x, sizeof(x)); _RET(value); } @@ -822,7 +817,7 @@ l_set_sw(void *ptr, PyObject *value, Py_ssize_t size) return NULL; memcpy(&field, ptr, sizeof(field)); field = SWAP_LONG(field); - field = (long)SET(field, val, size); + field = SET(long, field, val, size); field = SWAP_LONG(field); memcpy(ptr, &field, sizeof(field)); _RET(value); @@ -856,7 +851,7 @@ L_set(void *ptr, PyObject *value, Py_ssize_t size) if (get_ulong(value, &val) < 0) return NULL; memcpy(&x, ptr, sizeof(x)); - x = SET(x, val, size); + x = SET(unsigned long, x, val, size); memcpy(ptr, &x, sizeof(x)); _RET(value); } @@ -870,7 +865,7 @@ L_set_sw(void *ptr, PyObject *value, Py_ssize_t size) return NULL; memcpy(&field, ptr, sizeof(field)); field = SWAP_LONG(field); - field = (unsigned long)SET(field, val, size); + field = SET(unsigned long, field, val, size); field = SWAP_LONG(field); memcpy(ptr, &field, sizeof(field)); _RET(value); @@ -905,7 +900,7 @@ q_set(void *ptr, PyObject *value, Py_ssize_t size) if (get_longlong(value, &val) < 0) return NULL; memcpy(&x, ptr, sizeof(x)); - x = SET(x, val, size); + x = SET(PY_LONG_LONG, x, val, size); memcpy(ptr, &x, sizeof(x)); _RET(value); } @@ -919,7 +914,7 @@ q_set_sw(void *ptr, PyObject *value, Py_ssize_t size) return NULL; memcpy(&field, ptr, sizeof(field)); field = SWAP_8(field); - field = (PY_LONG_LONG)SET(field, val, size); + field = SET(PY_LONG_LONG, field, val, size); field = SWAP_8(field); memcpy(ptr, &field, sizeof(field)); _RET(value); @@ -952,7 +947,7 @@ Q_set(void *ptr, PyObject *value, Py_ssize_t size) if (get_ulonglong(value, &val) < 0) return NULL; memcpy(&x, ptr, sizeof(x)); - x = SET(x, val, size); + x = SET(PY_LONG_LONG, x, val, size); memcpy(ptr, &x, sizeof(x)); _RET(value); } @@ -966,7 +961,7 @@ Q_set_sw(void *ptr, PyObject *value, Py_ssize_t size) return NULL; memcpy(&field, ptr, sizeof(field)); field = SWAP_8(field); - field = (unsigned PY_LONG_LONG)SET(field, val, size); + field = SET(unsigned PY_LONG_LONG, field, val, size); field = SWAP_8(field); memcpy(ptr, &field, sizeof(field)); _RET(value);