From 54d97251fe2dd4939652a186bf703885e654b4d1 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Sat, 12 Sep 2020 13:57:31 -0500 Subject: modstruct: Improve compliance with python3 While checking whether we can enable -Wimplicit-fallthrough, I encountered a diagnostic in mp_binary_set_val_array_from_int which led to discovering the following bug: ``` >>> struct.pack("xb", 3) b'\x03\x03' ``` That is, the next value (3) was used as the value of a padding byte, while standard Python always fills "x" bytes with zeros. I initially thought this had to do with the unintentional fallthrough, but it doesn't. Instead, this code would relate to an array.array with a typecode of padding ('x'), which is ALSO not desktop Python compliant: ``` >>> array.array('x', (1, 2, 3)) array('x', [1, 0, 0]) ``` Possibly this is dead code that used to be shared between struct-setting and array-setting, but it no longer is. I also discovered that the argument list length for struct.pack and struct.pack_into were not checked, and that the length of binary data passed to array.array was not checked to be a multiple of the element size. I have corrected all of these to conform more closely to standard Python and revised some tests where necessary. Some tests for micropython-specific behavior that does not conform to standard Python and is not present in CircuitPython was deleted outright. --- py/binary.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'py/binary.c') diff --git a/py/binary.c b/py/binary.c index cd0f1aa4d..b85edba62 100644 --- a/py/binary.c +++ b/py/binary.c @@ -126,7 +126,6 @@ mp_obj_t mp_binary_get_val_array(char typecode, void *p, mp_uint_t index) { break; case BYTEARRAY_TYPECODE: case 'B': - case 'x': // value will be discarded val = ((unsigned char*)p)[index]; break; case 'h': @@ -330,7 +329,11 @@ void mp_binary_set_val(char struct_type, char val_type, mp_obj_t val_in, byte ** } } - mp_binary_set_int(MIN((size_t)size, sizeof(val)), struct_type == '>', p, val); + if (val_type == 'x') { + memset(p, 0, 1); + } else { + mp_binary_set_int(MIN((size_t)size, sizeof(val)), struct_type == '>', p, val); + } } void mp_binary_set_val_array(char typecode, void *p, mp_uint_t index, mp_obj_t val_in) { @@ -379,8 +382,6 @@ void mp_binary_set_val_array_from_int(char typecode, void *p, mp_uint_t index, m case 'B': ((unsigned char*)p)[index] = val; break; - case 'x': - ((unsigned char*)p)[index] = 0; case 'h': ((short*)p)[index] = val; break; -- cgit v1.2.3