Switch to a shared piece of code to compute start and length of a

buffer from start, end and length. The old code miscomputed length
leading to writing and reading from memory past the end of the buffer.
Consolidating the code should make it easier to get right everywhere.

4 files changed, 32 insertions, 72 deletions

diff --git a/shared-bindings/bitbangio/I2C.c b/shared-bindings/bitbangio/I2C.c
index e8153f392..6c2c4223f 100644
--- a/shared-bindings/bitbangio/I2C.c
+++ b/shared-bindings/bitbangio/I2C.c
@@ -30,6 +30,7 @@
 #include "shared-bindings/bitbangio/I2C.h"
 #include "shared-bindings/microcontroller/Pin.h"
 
+#include "lib/utils/buffer_helper.h"
 #include "lib/utils/context_manager_helpers.h"
 #include "py/mperrno.h"
 #include "py/runtime.h"
@@ -172,21 +173,14 @@ STATIC mp_obj_t bitbangio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_a
     check_lock(self);
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
-    int32_t end = args[ARG_end].u_int;
-    if (end < 0) {
-        end += bufinfo.len;
-    }
-    uint32_t start = args[ARG_start].u_int;
-    uint32_t len = end - start;
-    if ((uint32_t) end < start) {
-        len = 0;
-    } else if (len > bufinfo.len) {
-        len = bufinfo.len;
-    }
+
+    int32_t start = args[ARG_start].u_int;
+    uint32_t length = bufinfo.len;
+    normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
     uint8_t status = shared_module_bitbangio_i2c_read(self,
                                                       args[ARG_address].u_int,
                                                       ((uint8_t*)bufinfo.buf) + start,
-                                                      len);
+                                                      length);
     if (status != 0) {
         mp_raise_OSError(status);
     }
@@ -228,21 +222,13 @@ STATIC mp_obj_t bitbangio_i2c_writeto(size_t n_args, const mp_obj_t *pos_args, m
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
 
-    int32_t end = args[ARG_end].u_int;
-    if (end < 0) {
-        end += bufinfo.len;
-    }
-    uint32_t start = args[ARG_start].u_int;
-    uint32_t len = end - start;
-    if ((uint32_t) end < start) {
-        len = 0;
-    } else if (len > bufinfo.len) {
-        len = bufinfo.len;
-    }
+    int32_t start = args[ARG_start].u_int;
+    uint32_t length = bufinfo.len;
+    normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
 
     // do the transfer
     uint8_t status = shared_module_bitbangio_i2c_write(self, args[ARG_address].u_int,
-        ((uint8_t*) bufinfo.buf) + start, len, args[ARG_stop].u_bool);
+        ((uint8_t*) bufinfo.buf) + start, length, args[ARG_stop].u_bool);
     if (status != 0) {
         mp_raise_OSError(status);
     }
diff --git a/shared-bindings/bitbangio/SPI.c b/shared-bindings/bitbangio/SPI.c
index c973403f0..f8cf90628 100644
--- a/shared-bindings/bitbangio/SPI.c
+++ b/shared-bindings/bitbangio/SPI.c
@@ -185,6 +185,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(bitbangio_spi_unlock_obj, bitbangio_spi_obj_unlock);
 //|
 //|     Write the data contained in ``buf``. Requires the SPI being locked.
 //|
+// TODO(tannewt): Add support for start and end kwargs.
 STATIC mp_obj_t bitbangio_spi_write(mp_obj_t self_in, mp_obj_t wr_buf) {
     mp_buffer_info_t src;
     mp_get_buffer_raise(wr_buf, &src, MP_BUFFER_READ);
@@ -203,6 +204,7 @@ MP_DEFINE_CONST_FUN_OBJ_2(bitbangio_spi_write_obj, bitbangio_spi_write);
 //|
 //|     Read into the buffer specified by ``buf`` while writing zeroes. Requires the SPI being locked.
 //|
+// TODO(tannewt): Add support for start and end kwargs.
 STATIC mp_obj_t bitbangio_spi_readinto(size_t n_args, const mp_obj_t *args) {
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[1], &bufinfo, MP_BUFFER_WRITE);
diff --git a/shared-bindings/busio/I2C.c b/shared-bindings/busio/I2C.c
index 7383cb104..b1794fdfb 100644
--- a/shared-bindings/busio/I2C.c
+++ b/shared-bindings/busio/I2C.c
@@ -30,6 +30,7 @@
 #include "shared-bindings/microcontroller/Pin.h"
 #include "shared-bindings/busio/I2C.h"
 
+#include "lib/utils/buffer_helper.h"
 #include "lib/utils/context_manager_helpers.h"
 #include "py/runtime.h"
 //| .. currentmodule:: busio
@@ -187,18 +188,11 @@ STATIC mp_obj_t busio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_args,
 
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
-    int32_t end = args[ARG_end].u_int;
-    if (end < 0) {
-        end += bufinfo.len;
-    }
-    uint32_t start = args[ARG_start].u_int;
-    uint32_t len = end - start;
-    if ((uint32_t) end < start) {
-        len = 0;
-    } else if (len > bufinfo.len) {
-        len = bufinfo.len;
-    }
-    uint8_t status = common_hal_busio_i2c_read(self, args[ARG_address].u_int, ((uint8_t*)bufinfo.buf) + start, len);
+
+    int32_t start = args[ARG_start].u_int;
+    uint32_t length = bufinfo.len;
+    normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
+    uint8_t status = common_hal_busio_i2c_read(self, args[ARG_address].u_int, ((uint8_t*)bufinfo.buf) + start, length);
     if (status != 0) {
         mp_raise_OSError(status);
     }
@@ -241,21 +235,14 @@ STATIC mp_obj_t busio_i2c_writeto(size_t n_args, const mp_obj_t *pos_args, mp_ma
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
 
-    int32_t end = args[ARG_end].u_int;
-    if (end < 0) {
-        end += bufinfo.len;
-    }
-    uint32_t start = args[ARG_start].u_int;
-    uint32_t len = end - start;
-    if ((uint32_t) end < start) {
-        len = 0;
-    } else if (len > bufinfo.len) {
-        len = bufinfo.len;
-    }
+
+    int32_t start = args[ARG_start].u_int;
+    uint32_t length = bufinfo.len;
+    normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
 
     // do the transfer
     uint8_t status = common_hal_busio_i2c_write(self, args[ARG_address].u_int,
-        ((uint8_t*) bufinfo.buf) + start, len, args[ARG_stop].u_bool);
+        ((uint8_t*) bufinfo.buf) + start, length, args[ARG_stop].u_bool);
     if (status != 0) {
         mp_raise_OSError(status);
     }
diff --git a/shared-bindings/busio/SPI.c b/shared-bindings/busio/SPI.c
index d4d9ec345..0fb0c7ddf 100644
--- a/shared-bindings/busio/SPI.c
+++ b/shared-bindings/busio/SPI.c
@@ -32,6 +32,7 @@
 #include "shared-bindings/microcontroller/Pin.h"
 #include "shared-bindings/busio/SPI.h"
 
+#include "lib/utils/buffer_helper.h"
 #include "lib/utils/context_manager_helpers.h"
 #include "py/mperrno.h"
 #include "py/nlr.h"
@@ -217,19 +218,11 @@ STATIC mp_obj_t busio_spi_write(size_t n_args, const mp_obj_t *pos_args, mp_map_
 
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
-    int32_t end = args[ARG_end].u_int;
-    if (end < 0) {
-        end += bufinfo.len;
-    }
-    uint32_t start = args[ARG_start].u_int;
-    uint32_t len = end - start;
-    if ((uint32_t) end < start) {
-        len = 0;
-    } else if (len > bufinfo.len) {
-        len = bufinfo.len;
-    }
+    int32_t start = args[ARG_start].u_int;
+    uint32_t length = bufinfo.len;
+    normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
 
-    bool ok = common_hal_busio_spi_write(self, ((uint8_t*)bufinfo.buf) + start, len);
+    bool ok = common_hal_busio_spi_write(self, ((uint8_t*)bufinfo.buf) + start, length);
     if (!ok) {
         mp_raise_OSError(MP_EIO);
     }
@@ -262,19 +255,11 @@ STATIC mp_obj_t busio_spi_readinto(size_t n_args, const mp_obj_t *pos_args, mp_m
 
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
-    int32_t end = args[ARG_end].u_int;
-    if (end < 0) {
-        end += bufinfo.len;
-    }
-    uint32_t start = args[ARG_start].u_int;
-    uint32_t len = end - start;
-    if ((uint32_t) end < start) {
-        len = 0;
-    } else if (len > bufinfo.len) {
-        len = bufinfo.len;
-    }
+    int32_t start = args[ARG_start].u_int;
+    uint32_t length = bufinfo.len;
+    normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
 
-    bool ok = common_hal_busio_spi_read(self, ((uint8_t*)bufinfo.buf) + start, len, args[ARG_write_value].u_int);
+    bool ok = common_hal_busio_spi_read(self, ((uint8_t*)bufinfo.buf) + start, length, args[ARG_write_value].u_int);
     if (!ok) {
         mp_raise_OSError(MP_EIO);
     }