8000 vfs: Implement common helper for vfs_blockdev read and write. · micropython/micropython@7f74230 · GitHub
[go: up one dir, main page]
Skip to content

Commit 7f74230

Browse files
projectgusprojectgus
committed
vfs: Implement common helper for vfs_blockdev 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>
1 parent 4bc7276 commit 7f74230

File tree

3 files changed

+39
-51
lines changed

3 files changed

+39
-51
lines changed

‎extmod/vfs_blockdev.c

Lines changed: 24 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,6 @@
3232

3333
#if MICROPY_VFS
3434

35-
// Block device functions are expected to return 0 on success
36-
// and negative integer on errors. Check for positive integer
37-
// results as some callers (i.e. littlefs) will produce corrupt
38-
// results from these.
39-
static int mp_vfs_check_result(mp_obj_t ret) {
40-
if (ret == mp_const_none) {
41-
return 0;
42-
} else {
43-
int i = MP_OBJ_SMALL_INT_VALUE(ret);
44-
return i > 0 ? (-MP_EINVAL) : i;
45-
}
46-
}
47-
4835
void mp_vfs_blockdev_init(mp_vfs_blockdev_t *self, mp_obj_t bdev) {
4936
mp_load_method(bdev, MP_QSTR_readblocks, self->readblocks);
5037
mp_load_method_maybe(bdev, MP_QSTR_writeblocks, self->writeblocks);
@@ -59,27 +46,37 @@ void mp_vfs_blockdev_init(mp_vfs_blockdev_t *self, mp_obj_t bdev) {
5946
}
6047
}
6148

49+
// Helper function to minimise code size of read/write functions
50+
static int mp_vfs_blockdev_call_rw(mp_obj_t *args, bool ext_fn, size_t block_num, size_t block_off, size_t len, void *buf) {
51+
mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, len, buf};
52+
args[2] = MP_OBJ_NEW_SMALL_INT(block_num);
53+
args[3] = MP_OBJ_FROM_PTR(&ar);
54+
args[4] = MP_OBJ_NEW_SMALL_INT(block_off); // ignored for !ext_fn
55+
mp_obj_t ret = mp_call_method_n_kw(ext_fn ? 3 : 2, 0, args);
56+
57+
if (ret == mp_const_none) {
58+
return 0;
59+
} else {
60+
// Block device functions are expected to return 0 on success
61+
// and negative integer on errors. Check for positive integer
62+
// results as some callers (i.e. littlefs) will produce corrupt
63+
// results from these.
64+
int i = MP_OBJ_SMALL_INT_VALUE(ret);
65+
return i > 0 ? (-MP_EINVAL) : i;
66+
}
67+
}
68+
6269
int mp_vfs_blockdev_read(mp_vfs_blockdev_t *self, size_t block_num, size_t num_blocks, uint8_t *buf) {
6370
if (self->flags & MP_BLOCKDEV_FLAG_NATIVE) {
6471
mp_uint_t (*f)(uint8_t *, uint32_t, uint32_t) = (void *)(uintptr_t)self->readblocks[2];
6572
return f(buf, block_num, num_blocks);
6673
} else {
67-
mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, num_blocks *self->block_size, buf};
68-
self->readblocks[2] = MP_OBJ_NEW_SMALL_INT(block_num);
69-
self->readblocks[3] = MP_OBJ_FROM_PTR(&ar);
70-
mp_call_method_n_kw(2, 0, self->readblocks);
71-
// TODO handle error return
72-
return 0;
74+
return mp_vfs_blockdev_call_rw(self->readblocks, false, block_num, 0, num_blocks * self->block_size, buf);
7375
}
7476
}
7577

7678
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) {
77-
mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, len, buf};
78-
self->readblocks[2] = MP_OBJ_NEW_SMALL_INT(block_num);
79-
self->readblocks[3] = MP_OBJ_FROM_PTR(&ar);
80-
self->readblocks[4] = MP_OBJ_NEW_SMALL_INT(block_off);
81-
mp_obj_t ret = mp_call_method_n_kw(3, 0, self->readblocks);
82-
return mp_vfs_check_result(ret);
79+
return mp_vfs_blockdev_call_rw(self->readblocks, true, block_num, block_off, len, buf);
8380
}
8481

8582
int mp_vfs_blockdev_write(mp_vfs_blockdev_t *self, size_t block_num, size_t num_blocks, const uint8_t *buf) {
@@ -92,12 +89,7 @@ int mp_vfs_blockdev_write(mp_vfs_blockdev_t *self, size_t block_num, size_t num_
9289
mp_uint_t (*f)(const uint8_t *, uint32_t, uint32_t) = (void *)(uintptr_t)self->writeblocks[2];
9390
return f(buf, block_num, num_blocks);
9491
} else {
95-
mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, num_blocks *self->block_size, (void *)buf};
96-
self->writeblocks[2] = MP_OBJ_NEW_SMALL_INT(block_num);
97-
self->writeblocks[3] = MP_OBJ_FROM_PTR(&ar);
98-
mp_call_method_n_kw(2, 0, self->writeblocks);
99-
// TODO handle error return
100-
return 0;
92+
return mp_vfs_blockdev_call_rw(self->writeblocks, false, block_num, 0, num_blocks * self->block_size, (void *)buf);
10193
}
10294
}
10395

@@ -106,13 +98,7 @@ int mp_vfs_blockdev_write_ext(mp_vfs_blockdev_t *self, size_t block_num, size_t
10698
// read-only block device
10799
return -MP_EROFS;
108100
}
109-
110-
mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, len, (void *)buf};
111-
self->writeblocks[2] = MP_OBJ_NEW_SMALL_INT(block_num);
112-
self->writeblocks[3] = MP_OBJ_FROM_PTR(&ar);
113-
self->writeblocks[4] = MP_OBJ_NEW_SMALL_INT(block_off);
114-
mp_obj_t ret = mp_call_method_n_kw(3, 0, self->writeblocks);
115-
return mp_vfs_check_result(ret);
101+
return mp_vfs_blockdev_call_rw(self->writeblocks, true, block_num, block_off, len, (void *)buf);
116102
}
117103

118104
mp_obj_t mp_vfs_blockdev_ioctl(mp_vfs_blockdev_t *self, uintptr_t cmd, uintptr_t arg) {

‎tests/extmod/vfs_blockdev_invalid.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ def ioctl(self, op, arg):
5252

5353
def test(vfs_class):
5454
print(vfs_class)
55+
bdev.read_res = 0 # reset function results
56+
bdev.write_res = 0
5557

5658
vfs_class.mkfs(bdev)
5759
fs = vfs_class(bdev)
@@ -84,4 +86,4 @ F438 @ def test(vfs_class):
8486

8587

8688
test(vfs.VfsLfs2)
87-
test(vfs.VfsFat) # Looks like most failures of underlying device are ignored by VFAT currently
89+
test(vfs.VfsFat)

‎tests/extmod/vfs_blockdev_invalid.py.exp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,22 +105,22 @@ readblocks
105105
read 1 a
106106
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
107107
readblocks
108-
opened
108+
OSError [Errno 5] EIO
109109
readblocks
110-
read 1 a
111-
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
112110
readblocks
113-
opened
111+
OSError [Errno 5] EIO
114112
readblocks
115-
read 1 a
116-
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
113+
OSError [Errno 5] EIO
117114
readblocks
118-
opened
119115
readblocks
120-
read 1 a
121-
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
116+
OSError [Errno 5] EIO
122117
readblocks
123-
opened
118+
OSError [Errno 5] EIO
124119
readblocks
125-
read 1 a
126-
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
120+
readblocks
121+
OSError [Errno 5] EIO
122+
readblocks
123+
OSError [Errno 5] EIO
124+
readblocks
125+
readblocks
126+
OSError [Errno 5] EIO

0 commit comments

Comments
 (0)
0