extmod/vfs_blockdev: Implement common helper for read and write.

- Code size saving as all of these functions are very similar.
- Resolves the "TODO" of the plain read and write functions not propagating
  errors. An error in the underlying block device now causes VFatFs to
  return EIO, for example.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>

3 files changed, 40 insertions, 51 deletions

diff --git a/extmod/vfs_blockdev.c b/extmod/vfs_blockdev.c
index 05b71ea1d..a7c14b76e 100644
--- a/extmod/vfs_blockdev.c
+++ b/extmod/vfs_blockdev.c
@@ -32,19 +32,6 @@
 
 #if MICROPY_VFS
 
-// Block device functions are expected to return 0 on success
-// and negative integer on errors. Check for positive integer
-// results as some callers (i.e. littlefs) will produce corrupt
-// results from these.
-static int mp_vfs_check_result(mp_obj_t ret) {
-    if (ret == mp_const_none) {
-        return 0;
-    } else {
-        int i = MP_OBJ_SMALL_INT_VALUE(ret);
-        return i > 0 ? (-MP_EINVAL) : i;
-    }
-}
-
 void mp_vfs_blockdev_init(mp_vfs_blockdev_t *self, mp_obj_t bdev) {
     mp_load_method(bdev, MP_QSTR_readblocks, self->readblocks);
     mp_load_method_maybe(bdev, MP_QSTR_writeblocks, self->writeblocks);
@@ -59,27 +46,38 @@ void mp_vfs_blockdev_init(mp_vfs_blockdev_t *self, mp_obj_t bdev) {
     }
 }
 
+// Helper function to minimise code size of read/write functions
+// note the n_args argument is moved to the end for further code size reduction (args keep same position in caller and callee).
+static int mp_vfs_blockdev_call_rw(mp_obj_t *args, size_t block_num, size_t block_off, size_t len, void *buf, size_t n_args) {
+    mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, len, buf};
+    args[2] = MP_OBJ_NEW_SMALL_INT(block_num);
+    args[3] = MP_OBJ_FROM_PTR(&ar);
+    args[4] = MP_OBJ_NEW_SMALL_INT(block_off); // ignored for n_args == 2
+    mp_obj_t ret = mp_call_method_n_kw(n_args, 0, args);
+
+    if (ret == mp_const_none) {
+        return 0;
+    } else {
+        // Block device functions are expected to return 0 on success
+        // and negative integer on errors. Check for positive integer
+        // results as some callers (i.e. littlefs) will produce corrupt
+        // results from these.
+        int i = MP_OBJ_SMALL_INT_VALUE(ret);
+        return i > 0 ? (-MP_EINVAL) : i;
+    }
+}
+
 int mp_vfs_blockdev_read(mp_vfs_blockdev_t *self, size_t block_num, size_t num_blocks, uint8_t *buf) {
     if (self->flags & MP_BLOCKDEV_FLAG_NATIVE) {
         mp_uint_t (*f)(uint8_t *, uint32_t, uint32_t) = (void *)(uintptr_t)self->readblocks[2];
         return f(buf, block_num, num_blocks);
     } else {
-        mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, num_blocks *self->block_size, buf};
-        self->readblocks[2] = MP_OBJ_NEW_SMALL_INT(block_num);
-        self->readblocks[3] = MP_OBJ_FROM_PTR(&ar);
-        mp_call_method_n_kw(2, 0, self->readblocks);
-        // TODO handle error return
-        return 0;
+        return mp_vfs_blockdev_call_rw(self->readblocks, block_num, 0, num_blocks * self->block_size, buf, 2);
     }
 }
 
 int mp_vfs_blockdev_read_ext(mp_vfs_blockdev_t *self, size_t block_num, size_t block_off, size_t len, uint8_t *buf) {
-    mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, len, buf};
-    self->readblocks[2] = MP_OBJ_NEW_SMALL_INT(block_num);
-    self->readblocks[3] = MP_OBJ_FROM_PTR(&ar);
-    self->readblocks[4] = MP_OBJ_NEW_SMALL_INT(block_off);
-    mp_obj_t ret = mp_call_method_n_kw(3, 0, self->readblocks);
-    return mp_vfs_check_result(ret);
+    return mp_vfs_blockdev_call_rw(self->readblocks, block_num, block_off, len, buf, 3);
 }
 
 int mp_vfs_blockdev_write(mp_vfs_blockdev_t *self, size_t block_num, size_t num_blocks, const uint8_t *buf) {
@@ -92,12 +90,7 @@ int mp_vfs_blockdev_write(mp_vfs_blockdev_t *self, size_t block_num, size_t num_
         mp_uint_t (*f)(const uint8_t *, uint32_t, uint32_t) = (void *)(uintptr_t)self->writeblocks[2];
         return f(buf, block_num, num_blocks);
     } else {
-        mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, num_blocks *self->block_size, (void *)buf};
-        self->writeblocks[2] = MP_OBJ_NEW_SMALL_INT(block_num);
-        self->writeblocks[3] = MP_OBJ_FROM_PTR(&ar);
-        mp_call_method_n_kw(2, 0, self->writeblocks);
-        // TODO handle error return
-        return 0;
+        return mp_vfs_blockdev_call_rw(self->writeblocks, block_num, 0, num_blocks * self->block_size, (void *)buf, 2);
     }
 }
 
@@ -106,13 +99,7 @@ int mp_vfs_blockdev_write_ext(mp_vfs_blockdev_t *self, size_t block_num, size_t
         // read-only block device
         return -MP_EROFS;
     }
-
-    mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, len, (void *)buf};
-    self->writeblocks[2] = MP_OBJ_NEW_SMALL_INT(block_num);
-    self->writeblocks[3] = MP_OBJ_FROM_PTR(&ar);
-    self->writeblocks[4] = MP_OBJ_NEW_SMALL_INT(block_off);
-    mp_obj_t ret = mp_call_method_n_kw(3, 0, self->writeblocks);
-    return mp_vfs_check_result(ret);
+    return mp_vfs_blockdev_call_rw(self->writeblocks, block_num, block_off, len, (void *)buf, 3);
 }
 
 mp_obj_t mp_vfs_blockdev_ioctl(mp_vfs_blockdev_t *self, uintptr_t cmd, uintptr_t arg) {
diff --git a/tests/extmod/vfs_blockdev_invalid.py b/tests/extmod/vfs_blockdev_invalid.py
index 80afc6414..9b12746b2 100644
--- a/tests/extmod/vfs_blockdev_invalid.py
+++ b/tests/extmod/vfs_blockdev_invalid.py
@@ -52,6 +52,8 @@ except MemoryError:
 
 def test(vfs_class):
     print(vfs_class)
+    bdev.read_res = 0  # reset function results
+    bdev.write_res = 0
 
     vfs_class.mkfs(bdev)
     fs = vfs_class(bdev)
@@ -84,4 +86,4 @@ def test(vfs_class):
 
 
 test(vfs.VfsLfs2)
-test(vfs.VfsFat)  # Looks like most failures of underlying device are ignored by VFAT currently
+test(vfs.VfsFat)
diff --git a/tests/extmod/vfs_blockdev_invalid.py.exp b/tests/extmod/vfs_blockdev_invalid.py.exp
index 992111719..64749da11 100644
--- a/tests/extmod/vfs_blockdev_invalid.py.exp
+++ b/tests/extmod/vfs_blockdev_invalid.py.exp
@@ -105,22 +105,22 @@ readblocks
 read 1 a
 read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 readblocks
-opened
+OSError [Errno 5] EIO
 readblocks
-read 1 a
-read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 readblocks
-opened
+OSError [Errno 5] EIO
 readblocks
-read 1 a
-read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+OSError [Errno 5] EIO
 readblocks
-opened
 readblocks
-read 1 a
-read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+OSError [Errno 5] EIO
 readblocks
-opened
+OSError [Errno 5] EIO
 readblocks
-read 1 a
-read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+readblocks
+OSError [Errno 5] EIO
+readblocks
+OSError [Errno 5] EIO
+readblocks
+readblocks
+OSError [Errno 5] EIO