Skip to content

Commit 238e121

Browse files
committed
protocols: Allow them to be (optionally) type-safe
Protocols are nice, but there is no way for C code to verify whether a type's "protocol" structure actually implements some particular protocol. As a result, you can pass an object that implements the "vfs" protocol to one that expects the "stream" protocol, and the opposite of awesomeness ensues. This patch adds an OPTIONAL (but enabled by default) protocol identifier as the first member of any protocol structure. This identifier is simply a unique QSTR chosen by the protocol designer and used by each protocol implementer. When checking for protocol support, instead of just checking whether the object's type has a non-NULL protocol field, use `mp_proto_get` which implements the protocol check when possible. The existing protocols are now named: protocol_framebuf protocol_i2c protocol_pin protocol_stream protocol_spi protocol_vfs (most of these are unused in CP and are just inherited from MP; vfs and stream are definitely used though) I did not find any crashing examples, but here's one to give a flavor of what is improved, using `micropython_coverage`. Before the change, the vfs "ioctl" protocol is invoked, and the result is not intelligible as json (but it could have resulted in a hard fault, potentially): >>> import uos, ujson >>> u = uos.VfsPosix('/tmp') >>> ujson.load(u) Traceback (most recent call last): File "<stdin>", line 1, in <module> ValueError: syntax error in JSON After the change, the vfs object is correctly detected as not supporting the stream protocol: >>> ujson.load(p) Traceback (most recent call last): File "<stdin>", line 1, in <module> OSError: stream operation not supported
1 parent 15886b1 commit 238e121

38 files changed

+170
-36
lines changed

extmod/machine_i2c.c

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ MP_DEFINE_CONST_FUN_OBJ_KW(machine_i2c_init_obj, 1, machine_i2c_obj_init);
318318

319319
STATIC mp_obj_t machine_i2c_scan(mp_obj_t self_in) {
320320
mp_obj_base_t *self = MP_OBJ_TO_PTR(self_in);
321-
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)self->type->protocol;
321+
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)mp_proto_get(self, QSTR_protocol_i2c);
322322
mp_obj_t list = mp_obj_new_list(0, NULL);
323323
// 7-bit addresses 0b0000xxx and 0b1111xxx are reserved
324324
for (int addr = 0x08; addr < 0x78; ++addr) {
@@ -333,7 +333,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(machine_i2c_scan_obj, machine_i2c_scan);
333333

334334
STATIC mp_obj_t machine_i2c_start(mp_obj_t self_in) {
335335
mp_obj_base_t *self = (mp_obj_base_t*)MP_OBJ_TO_PTR(self_in);
336-
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)self->type->protocol;
336+
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)mp_proto_get(self, QSTR_protocol_i2c);
337337
if (i2c_p->start == NULL) {
338338
mp_raise_msg(&mp_type_OSError, translate("I2C operation not supported"));
339339
}
@@ -347,7 +347,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(machine_i2c_start_obj, machine_i2c_start);
347347

348348
STATIC mp_obj_t machine_i2c_stop(mp_obj_t self_in) {
349349
mp_obj_base_t *self = (mp_obj_base_t*)MP_OBJ_TO_PTR(self_in);
350-
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)self->type->protocol;
350+
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)mp_proto_get(self, QSTR_protocol_i2c);
351351
if (i2c_p->stop == NULL) {
352352
mp_raise_msg(&mp_type_OSError, translate("I2C operation not supported"));
353353
}
@@ -361,7 +361,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(machine_i2c_stop_obj, machine_i2c_stop);
361361

362362
STATIC mp_obj_t machine_i2c_readinto(size_t n_args, const mp_obj_t *args) {
363363
mp_obj_base_t *self = (mp_obj_base_t*)MP_OBJ_TO_PTR(args[0]);
364-
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)self->type->protocol;
364+
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)mp_proto_get(self, QSTR_protocol_i2c);
365365
if (i2c_p->read == NULL) {
366366
mp_raise_msg(&mp_type_OSError, translate("I2C operation not supported"));
367367
}
@@ -385,7 +385,7 @@ MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(machine_i2c_readinto_obj, 2, 3, machine_i2c_
385385

386386
STATIC mp_obj_t machine_i2c_write(mp_obj_t self_in, mp_obj_t buf_in) {
387387
mp_obj_base_t *self = (mp_obj_base_t*)MP_OBJ_TO_PTR(self_in);
388-
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)self->type->protocol;
388+
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)mp_proto_get(self, QSTR_protocol_i2c);
389389
if (i2c_p->write == NULL) {
390390
mp_raise_msg(&mp_type_OSError, translate("I2C operation not supported"));
391391
}
@@ -407,7 +407,7 @@ MP_DEFINE_CONST_FUN_OBJ_2(machine_i2c_write_obj, machine_i2c_write);
407407

408408
STATIC mp_obj_t machine_i2c_readfrom(size_t n_args, const mp_obj_t *args) {
409409
mp_obj_base_t *self = (mp_obj_base_t*)MP_OBJ_TO_PTR(args[0]);
410-
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)self->type->protocol;
410+
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)mp_proto_get(self, QSTR_protocol_i2c);
411411
mp_int_t addr = mp_obj_get_int(args[1]);
412412
vstr_t vstr;
413413
vstr_init_len(&vstr, mp_obj_get_int(args[2]));
@@ -422,7 +422,7 @@ MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(machine_i2c_readfrom_obj, 3, 4, machine_i2c_
422422

423423
STATIC mp_obj_t machine_i2c_readfrom_into(size_t n_args, const mp_obj_t *args) {
424424
mp_obj_base_t *self = (mp_obj_base_t*)MP_OBJ_TO_PTR(args[0]);
425-
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)self->type->protocol;
425+
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)mp_proto_get(self, QSTR_protocol_i2c);
426426
mp_int_t addr = mp_obj_get_int(args[1]);
427427
mp_buffer_info_t bufinfo;
428428
mp_get_buffer_raise(args[2], &bufinfo, MP_BUFFER_WRITE);
@@ -437,7 +437,7 @@ MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(machine_i2c_readfrom_into_obj, 3, 4, machine
437437

438438
STATIC mp_obj_t machine_i2c_writeto(size_t n_args, const mp_obj_t *args) {
439439
mp_obj_base_t *self = (mp_obj_base_t*)MP_OBJ_TO_PTR(args[0]);
440-
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)self->type->protocol;
440+
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)mp_proto_get(self, QSTR_protocol_i2c);
441441
mp_int_t addr = mp_obj_get_int(args[1]);
442442
mp_buffer_info_t bufinfo;
443443
mp_get_buffer_raise(args[2], &bufinfo, MP_BUFFER_READ);
@@ -453,7 +453,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(machine_i2c_writeto_obj, 3, 4, machin
453453

454454
STATIC int read_mem(mp_obj_t self_in, uint16_t addr, uint32_t memaddr, uint8_t addrsize, uint8_t *buf, size_t len) {
455455
mp_obj_base_t *self = (mp_obj_base_t*)MP_OBJ_TO_PTR(self_in);
456-
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)self->type->protocol;
456+
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)mp_proto_get(self, QSTR_protocol_i2c);
457457
uint8_t memaddr_buf[4];
458458
size_t memaddr_len = 0;
459459
for (int16_t i = addrsize - 8; i >= 0; i -= 8) {
@@ -473,7 +473,7 @@ STATIC int read_mem(mp_obj_t self_in, uint16_t addr, uint32_t memaddr, uint8_t a
473473

474474
STATIC int write_mem(mp_obj_t self_in, uint16_t addr, uint32_t memaddr, uint8_t addrsize, const uint8_t *buf, size_t len) {
475475
mp_obj_base_t *self = (mp_obj_base_t*)MP_OBJ_TO_PTR(self_in);
476-
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)self->type->protocol;
476+
mp_machine_i2c_p_t *i2c_p = (mp_machine_i2c_p_t*)mp_proto_get(self, QSTR_protocol_i2c);
477477

478478
// need some memory to create the buffer to send; try to use stack if possible
479479
uint8_t buf2_stack[MAX_MEMADDR_SIZE + BUF_STACK_SIZE];
@@ -621,6 +621,7 @@ int mp_machine_soft_i2c_write(mp_obj_base_t *self_in, const uint8_t *src, size_t
621621
}
622622

623623
STATIC const mp_machine_i2c_p_t mp_machine_soft_i2c_p = {
624+
MP_PROTO_IMPLEMENT(MP_QSTR_protocol_i2c)
624625
.start = (int(*)(mp_obj_base_t*))mp_hal_i2c_start,
625626
.stop = (int(*)(mp_obj_base_t*))mp_hal_i2c_stop,
626627
.read = mp_machine_soft_i2c_read,

extmod/machine_i2c.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,12 @@
2727
#define MICROPY_INCLUDED_EXTMOD_MACHINE_I2C_H
2828

2929
#include "py/obj.h"
30+
#include "py/proto.h"
3031

3132
// I2C protocol
3233
// the first 4 methods can be NULL, meaning operation is not supported
3334
typedef struct _mp_machine_i2c_p_t {
35+
MP_PROTOCOL_HEAD
3436
int (*start)(mp_obj_base_t *obj);
3537
int (*stop)(mp_obj_base_t *obj);
3638
int (*read)(mp_obj_base_t *obj, uint8_t *dest, size_t len, bool nack);

extmod/machine_pinbase.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ mp_uint_t pinbase_ioctl(mp_obj_t obj, mp_uint_t request, uintptr_t arg, int *err
7474
}
7575

7676
STATIC const mp_pin_p_t pinbase_pin_p = {
77+
MP_PROTO_IMPLEMENT(MP_QSTR_protocol_pin)
7778
.ioctl = pinbase_ioctl,
7879
};
7980

extmod/machine_signal.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,7 @@ STATIC mp_obj_t signal_make_new(const mp_obj_type_t *type, size_t n_args, const
4747
bool invert = false;
4848

4949
#if defined(MICROPY_PY_MACHINE_PIN_MAKE_NEW)
50-
mp_pin_p_t *pin_p = NULL;
51-
52-
if (MP_OBJ_IS_OBJ(pin)) {
53-
mp_obj_base_t *pin_base = (mp_obj_base_t*)MP_OBJ_TO_PTR(args[0]);
54-
pin_p = (mp_pin_p_t*)pin_base->type->protocol;
55-
}
50+
mp_pin_p_t *pin_p = (mp_pin_t*)mp_proto_get(QSTR_pin_protocol, pin);
5651

5752
if (pin_p == NULL) {
5853
// If first argument isn't a Pin-like object, we filter out "invert"
@@ -170,6 +165,7 @@ STATIC const mp_rom_map_elem_t signal_locals_dict_table[] = {
170165
STATIC MP_DEFINE_CONST_DICT(signal_locals_dict, signal_locals_dict_table);
171166

172167
STATIC const mp_pin_p_t signal_pin_p = {
168+
MP_PROTO_IMPLEMENT(MP_QSTR_protocol_pin)
173169
.ioctl = signal_ioctl,
174170
};
175171

extmod/machine_spi.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,15 @@ mp_obj_t mp_machine_spi_make_new(const mp_obj_type_t *type, size_t n_args, const
6767

6868
STATIC mp_obj_t machine_spi_init(size_t n_args, const mp_obj_t *args, mp_map_t *kw_args) {
6969
mp_obj_base_t *s = (mp_obj_base_t*)MP_OBJ_TO_PTR(args[0]);
70-
mp_machine_spi_p_t *spi_p = (mp_machine_spi_p_t*)s->type->protocol;
70+
mp_machine_spi_p_t *spi_p = (mp_machine_spi_p_t*)mp_proto_get(QSTR_protocol_spi, s);
7171
spi_p->init(s, n_args - 1, args + 1, kw_args);
7272
return mp_const_none;
7373
}
7474
STATIC MP_DEFINE_CONST_FUN_OBJ_KW(machine_spi_init_obj, 1, machine_spi_init);
7575

7676
STATIC mp_obj_t machine_spi_deinit(mp_obj_t self) {
7777
mp_obj_base_t *s = (mp_obj_base_t*)MP_OBJ_TO_PTR(self);
78-
mp_machine_spi_p_t *spi_p = (mp_machine_spi_p_t*)s->type->protocol;
78+
mp_machine_spi_p_t *spi_p = (mp_machine_spi_p_t*)mp_proto_get(QSTR_protocol_spi, s);
7979
if (spi_p->deinit != NULL) {
8080
spi_p->deinit(s);
8181
}
@@ -85,7 +85,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_1(machine_spi_deinit_obj, machine_spi_deinit);
8585

8686
STATIC void mp_machine_spi_transfer(mp_obj_t self, size_t len, const void *src, void *dest) {
8787
mp_obj_base_t *s = (mp_obj_base_t*)MP_OBJ_TO_PTR(self);
88-
mp_machine_spi_p_t *spi_p = (mp_machine_spi_p_t*)s->type->protocol;
88+
mp_machine_spi_p_t *spi_p = (mp_machine_spi_p_t*)mp_proto_get(QSTR_protocol_spi, s);
8989
spi_p->transfer(s, len, src, dest);
9090
}
9191

@@ -268,6 +268,7 @@ STATIC void mp_machine_soft_spi_transfer(mp_obj_base_t *self_in, size_t len, con
268268
}
269269

270270
const mp_machine_spi_p_t mp_machine_soft_spi_p = {
271+
MP_PROTO_IMPLEMENT(MP_QSTR_protocol_spi)
271272
.init = mp_machine_soft_spi_init,
272273
.deinit = NULL,
273274
.transfer = mp_machine_soft_spi_transfer,

extmod/machine_spi.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@
2727
#define MICROPY_INCLUDED_EXTMOD_MACHINE_SPI_H
2828

2929
#include "py/obj.h"
30+
#include "py/proto.h"
3031
#include "py/mphal.h"
3132
#include "drivers/bus/spi.h"
3233

3334
// SPI protocol
3435
typedef struct _mp_machine_spi_p_t {
36+
MP_PROTOCOL_HEAD
3537
void (*init)(mp_obj_base_t *obj, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args);
3638
void (*deinit)(mp_obj_base_t *obj); // can be NULL
3739
void (*transfer)(mp_obj_base_t *obj, size_t len, const uint8_t *src, uint8_t *dest);

extmod/modframebuf.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <string.h>
2929

3030
#include "py/runtime.h"
31+
#include "py/proto.h"
3132

3233
#if MICROPY_PY_FRAMEBUF
3334

@@ -46,6 +47,7 @@ typedef uint32_t (*getpixel_t)(const mp_obj_framebuf_t*, int, int);
4647
typedef void (*fill_rect_t)(const mp_obj_framebuf_t *, int, int, int, int, uint32_t);
4748

4849
typedef struct _mp_framebuf_p_t {
50+
MP_PROTOCOL_HEAD
4951
setpixel_t setpixel;
5052
getpixel_t getpixel;
5153
fill_rect_t fill_rect;
@@ -227,13 +229,13 @@ STATIC void gs8_fill_rect(const mp_obj_framebuf_t *fb, int x, int y, int w, int
227229
}
228230

229231
STATIC mp_framebuf_p_t formats[] = {
230-
[FRAMEBUF_MVLSB] = {mvlsb_setpixel, mvlsb_getpixel, mvlsb_fill_rect},
231-
[FRAMEBUF_RGB565] = {rgb565_setpixel, rgb565_getpixel, rgb565_fill_rect},
232-
[FRAMEBUF_GS2_HMSB] = {gs2_hmsb_setpixel, gs2_hmsb_getpixel, gs2_hmsb_fill_rect},
233-
[FRAMEBUF_GS4_HMSB] = {gs4_hmsb_setpixel, gs4_hmsb_getpixel, gs4_hmsb_fill_rect},
234-
[FRAMEBUF_GS8] = {gs8_setpixel, gs8_getpixel, gs8_fill_rect},
235-
[FRAMEBUF_MHLSB] = {mono_horiz_setpixel, mono_horiz_getpixel, mono_horiz_fill_rect},
236-
[FRAMEBUF_MHMSB] = {mono_horiz_setpixel, mono_horiz_getpixel, mono_horiz_fill_rect},
232+
[FRAMEBUF_MVLSB] = {MP_PROTO_IMPLEMENT(MP_QSTR_protocol_framebuf) mvlsb_setpixel, mvlsb_getpixel, mvlsb_fill_rect},
233+
[FRAMEBUF_RGB565] = {MP_PROTO_IMPLEMENT(MP_QSTR_protocol_framebuf) rgb565_setpixel, rgb565_getpixel, rgb565_fill_rect},
234+
[FRAMEBUF_GS2_HMSB] = {MP_PROTO_IMPLEMENT(MP_QSTR_protocol_framebuf) gs2_hmsb_setpixel, gs2_hmsb_getpixel, gs2_hmsb_fill_rect},
235+
[FRAMEBUF_GS4_HMSB] = {MP_PROTO_IMPLEMENT(MP_QSTR_protocol_framebuf) gs4_hmsb_setpixel, gs4_hmsb_getpixel, gs4_hmsb_fill_rect},
236+
[FRAMEBUF_GS8] = {MP_PROTO_IMPLEMENT(MP_QSTR_protocol_framebuf) gs8_setpixel, gs8_getpixel, gs8_fill_rect},
237+
[FRAMEBUF_MHLSB] = {MP_PROTO_IMPLEMENT(MP_QSTR_protocol_framebuf) mono_horiz_setpixel, mono_horiz_getpixel, mono_horiz_fill_rect},
238+
[FRAMEBUF_MHMSB] = {MP_PROTO_IMPLEMENT(MP_QSTR_protocol_framebuf) mono_horiz_setpixel, mono_horiz_getpixel, mono_horiz_fill_rect},
237239
};
238240

239241
static inline void setpixel(const mp_obj_framebuf_t *fb, int x, int y, uint32_t col) {

extmod/modlwip.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,6 +1261,7 @@ STATIC const mp_rom_map_elem_t lwip_socket_locals_dict_table[] = {
12611261
STATIC MP_DEFINE_CONST_DICT(lwip_socket_locals_dict, lwip_socket_locals_dict_table);
12621262

12631263
STATIC const mp_stream_p_t lwip_socket_stream_p = {
1264+
MP_PROTO_IMPLEMENT(MP_QSTR_protocol_stream)
12641265
.read = lwip_socket_read,
12651266
.write = lwip_socket_write,
12661267
.ioctl = lwip_socket_ioctl,

extmod/modussl_axtls.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ STATIC const mp_rom_map_elem_t ussl_socket_locals_dict_table[] = {
221221
STATIC MP_DEFINE_CONST_DICT(ussl_socket_locals_dict, ussl_socket_locals_dict_table);
222222

223223
STATIC const mp_stream_p_t ussl_socket_stream_p = {
224+
MP_PROTO_IMPLEMENT(MP_QSTR_protocol_stream)
224225
.read = socket_read,
225226
.write = socket_write,
226227
.ioctl = socket_ioctl,

extmod/modussl_mbedtls.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ STATIC const mp_rom_map_elem_t ussl_socket_locals_dict_table[] = {
305305
STATIC MP_DEFINE_CONST_DICT(ussl_socket_locals_dict, ussl_socket_locals_dict_table);
306306

307307
STATIC const mp_stream_p_t ussl_socket_stream_p = {
308+
MP_PROTO_IMPLEMENT(MP_QSTR_protocol_stream)
308309
.read = socket_read,
309310
.write = socket_write,
310311
.ioctl = socket_ioctl,

0 commit comments

Comments
 (0)