summaryrefslogtreecommitdiff
path: root/py/binary.c
diff options
context:
space:
mode:
authorJeff Epler <jepler@gmail.com>2020-09-12 13:57:31 -0500
committerJeff Epler <jepler@gmail.com>2020-09-12 14:07:23 -0500
commit54d97251fe2dd4939652a186bf703885e654b4d1 (patch)
treedaa39c435669778a4c1b02dbc4c47e087fd80db6 /py/binary.c
parentb24d3b886af1d4fbbe481961a3b3215fcc0ba799 (diff)
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.
Diffstat (limited to 'py/binary.c')
-rw-r--r--py/binary.c9
1 files changed, 5 insertions, 4 deletions
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;