Merge pull request adafruit#4620 from tannewt/fix_struct_pack

jepler · web-flow · commit 18548fc5d252 · 2021-04-19T20:43:20.000-05:00
Fix struct.pack with padding bytes
diff --git a/py/binary.c b/py/binary.c
@@ -371,11 +371,7 @@ void mp_binary_set_val(char struct_type, char val_type, mp_obj_t val_in, byte **
         }
     }
 
-    if (val_type == 'x') {
-        memset(p, 0, 1);
-    } else {
-        mp_binary_set_int(MIN((size_t)size, sizeof(val)), struct_type == '>', p, val);
-    }
+    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) {
diff --git a/py/modstruct.c b/py/modstruct.c
@@ -214,9 +214,11 @@ STATIC void struct_pack_into_internal(mp_obj_t fmt_in, byte *p, size_t n_args, c
             p += cnt;
         } else {
             while (cnt--) {
-                mp_binary_set_val(fmt_type, *fmt, args[i], &p);
                 // Pad bytes don't have a corresponding argument.
-                if (*fmt != 'x') {
+                if (*fmt == 'x') {
+                    mp_binary_set_val(fmt_type, *fmt, MP_OBJ_NEW_SMALL_INT(0), &p);
+                } else {
+                    mp_binary_set_val(fmt_type, *fmt, args[i], &p);
                     i++;
                 }
             }
diff --git a/shared-module/struct/__init__.c b/shared-module/struct/__init__.c
@@ -153,9 +153,11 @@ void shared_modules_struct_pack_into(mp_obj_t fmt_in, byte *p, byte *end_p, size
             p += sz;
         } else {
             while (sz--) {
-                mp_binary_set_val(fmt_type, *fmt, args[i], &p);
                 // Pad bytes don't have a corresponding argument.
-                if (*fmt != 'x') {
+                if (*fmt == 'x') {
+                    mp_binary_set_val(fmt_type, *fmt, MP_OBJ_NEW_SMALL_INT(0), &p);
+                } else {
+                    mp_binary_set_val(fmt_type, *fmt, args[i], &p);
                     i++;
                 }
             }
diff --git a/tests/basics/struct1.py b/tests/basics/struct1.py
@@ -121,3 +121,5 @@
 
 # check padding bytes
 print(struct.pack("xb", 3))
+# Make sure pack doesn't reuse a larger value and error
+print(struct.pack("xH", 0x100))

Original file line number	Diff line number	Diff line change
`@@ -371,11 +371,7 @@ void mp_binary_set_val(char struct_type, char val_type, mp_obj_t val_in, byte **`
`371`	`371`	`}`
`372`	`372`	`}`
`373`	`373`
`374`		`- if (val_type == 'x') {`
`375`		`- memset(p, 0, 1);`
`376`		`- } else {`
`377`		`- mp_binary_set_int(MIN((size_t)size, sizeof(val)), struct_type == '>', p, val);`
`378`		`- }`
	`374`	`+ mp_binary_set_int(MIN((size_t)size, sizeof(val)), struct_type == '>', p, val);`
`379`	`375`	`}`
`380`	`376`
`381`	`377`	`void mp_binary_set_val_array(char typecode, void *p, mp_uint_t index, mp_obj_t val_in) {`
Original file line number	Diff line number	Diff line change
`@@ -214,9 +214,11 @@ STATIC void struct_pack_into_internal(mp_obj_t fmt_in, byte *p, size_t n_args, c`
`214`	`214`	`p += cnt;`
`215`	`215`	`} else {`
`216`	`216`	`while (cnt--) {`
`217`		`- mp_binary_set_val(fmt_type, *fmt, args[i], &p);`
`218`	`217`	`// Pad bytes don't have a corresponding argument.`
`219`		`- if (*fmt != 'x') {`
	`218`	`+ if (*fmt == 'x') {`
	`219`	`+ mp_binary_set_val(fmt_type, *fmt, MP_OBJ_NEW_SMALL_INT(0), &p);`
	`220`	`+ } else {`
	`221`	`+ mp_binary_set_val(fmt_type, *fmt, args[i], &p);`
`220`	`222`	`i++;`
`221`	`223`	`}`
`222`	`224`	`}`
Original file line number	Diff line number	Diff line change
`@@ -153,9 +153,11 @@ void shared_modules_struct_pack_into(mp_obj_t fmt_in, byte p, byte end_p, size`
`153`	`153`	`p += sz;`
`154`	`154`	`} else {`
`155`	`155`	`while (sz--) {`
`156`		`- mp_binary_set_val(fmt_type, *fmt, args[i], &p);`
`157`	`156`	`// Pad bytes don't have a corresponding argument.`
`158`		`- if (*fmt != 'x') {`
	`157`	`+ if (*fmt == 'x') {`
	`158`	`+ mp_binary_set_val(fmt_type, *fmt, MP_OBJ_NEW_SMALL_INT(0), &p);`
	`159`	`+ } else {`
	`160`	`+ mp_binary_set_val(fmt_type, *fmt, args[i], &p);`
`159`	`161`	`i++;`
`160`	`162`	`}`
`161`	`163`	`}`