From d361fc52a04f6f22381429606f0b3ad2875afc22 Mon Sep 17 00:00:00 2001 From: Martine Lenders Date: Wed, 5 Jun 2019 12:25:12 +0200 Subject: [PATCH 1/5] tsrb: change input type to `uint8_t` Having the input type `char` makes the output of `tsrb_get_one()` incomparable to the input of `tsrb_add_one()`. By changing it to `uint8_t` we not only definitively fix it to an octet, but also ensure that the input and output are the same. --- sys/include/tsrb.h | 15 +++++++++------ sys/tsrb/tsrb.c | 10 +++++----- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/sys/include/tsrb.h b/sys/include/tsrb.h index 0dcc04730f..adb5968cd6 100644 --- a/sys/include/tsrb.h +++ b/sys/include/tsrb.h @@ -28,6 +28,7 @@ #include #include +#include #ifdef __cplusplus extern "C" { @@ -37,7 +38,7 @@ extern "C" { * @brief thread-safe ringbuffer struct */ typedef struct tsrb { - char *buf; /**< Buffer to operate on. */ + uint8_t *buf; /**< Buffer to operate on. */ unsigned int size; /**< Size of buffer, must be power of 2. */ volatile unsigned reads; /**< total number of reads */ volatile unsigned writes; /**< total number of writes */ @@ -46,7 +47,9 @@ typedef struct tsrb { /** * @brief Static initializer */ -#define TSRB_INIT(BUF) { (BUF), sizeof (BUF), 0, 0 } +/* XXX remove this implicit cast as soon as possible (requires API change + * to isrpipe)*/ +#define TSRB_INIT(BUF) { (uint8_t *)(BUF), sizeof (BUF), 0, 0 } /** * @brief Initialize a tsrb. @@ -54,7 +57,7 @@ typedef struct tsrb { * @param[in] buffer Buffer to use by tsrb. * @param[in] bufsize `sizeof (buffer)`, must be power of 2. */ -static inline void tsrb_init(tsrb_t *rb, char *buffer, unsigned bufsize) +static inline void tsrb_init(tsrb_t *rb, uint8_t *buffer, unsigned bufsize) { /* make sure bufsize is a power of two. * http://www.exploringbinary.com/ten-ways-to-check-if-an-integer-is-a-power-of-two-in-c/ @@ -125,7 +128,7 @@ int tsrb_get_one(tsrb_t *rb); * @param[in] n max number of bytes to write to @p dst * @return nr of bytes written to @p dst */ -int tsrb_get(tsrb_t *rb, char *dst, size_t n); +int tsrb_get(tsrb_t *rb, uint8_t *dst, size_t n); /** * @brief Drop bytes from ringbuffer @@ -142,7 +145,7 @@ int tsrb_drop(tsrb_t *rb, size_t n); * @return 0 on success * @return -1 if no space available */ -int tsrb_add_one(tsrb_t *rb, char c); +int tsrb_add_one(tsrb_t *rb, uint8_t c); /** * @brief Add bytes to ringbuffer @@ -151,7 +154,7 @@ int tsrb_add_one(tsrb_t *rb, char c); * @param[in] n max number of bytes to read from @p src * @return nr of bytes read from @p src */ -int tsrb_add(tsrb_t *rb, const char *src, size_t n); +int tsrb_add(tsrb_t *rb, const uint8_t *src, size_t n); #ifdef __cplusplus } diff --git a/sys/tsrb/tsrb.c b/sys/tsrb/tsrb.c index 9e1a0082e7..f53e87cc02 100644 --- a/sys/tsrb/tsrb.c +++ b/sys/tsrb/tsrb.c @@ -19,12 +19,12 @@ #include "tsrb.h" -static void _push(tsrb_t *rb, char c) +static void _push(tsrb_t *rb, uint8_t c) { rb->buf[rb->writes++ & (rb->size - 1)] = c; } -static char _pop(tsrb_t *rb) +static uint8_t _pop(tsrb_t *rb) { return rb->buf[rb->reads++ & (rb->size - 1)]; } @@ -39,7 +39,7 @@ int tsrb_get_one(tsrb_t *rb) } } -int tsrb_get(tsrb_t *rb, char *dst, size_t n) +int tsrb_get(tsrb_t *rb, uint8_t *dst, size_t n) { size_t tmp = n; while (tmp && !tsrb_empty(rb)) { @@ -59,7 +59,7 @@ int tsrb_drop(tsrb_t *rb, size_t n) return (n - tmp); } -int tsrb_add_one(tsrb_t *rb, char c) +int tsrb_add_one(tsrb_t *rb, uint8_t c) { if (!tsrb_full(rb)) { _push(rb, c); @@ -70,7 +70,7 @@ int tsrb_add_one(tsrb_t *rb, char c) } } -int tsrb_add(tsrb_t *rb, const char *src, size_t n) +int tsrb_add(tsrb_t *rb, const uint8_t *src, size_t n) { size_t tmp = n; while (tmp && !tsrb_full(rb)) { From fb19b514f4df804863fbbac136cdd3294163d409 Mon Sep 17 00:00:00 2001 From: Martine Lenders Date: Wed, 5 Jun 2019 12:50:38 +0200 Subject: [PATCH 2/5] tsrb: remove now unnecessary cast Both `unsigned char` and `uint8_t` are unsigned (and the same type on most platforms) so this cast became redundant. --- sys/tsrb/tsrb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sys/tsrb/tsrb.c b/sys/tsrb/tsrb.c index f53e87cc02..de7900e37a 100644 --- a/sys/tsrb/tsrb.c +++ b/sys/tsrb/tsrb.c @@ -32,7 +32,7 @@ static uint8_t _pop(tsrb_t *rb) int tsrb_get_one(tsrb_t *rb) { if (!tsrb_empty(rb)) { - return (unsigned char)_pop(rb); + return _pop(rb); } else { return -1; From dfc8bbd96cd22bc49a118a1bdc7bfe34983c4ab6 Mon Sep 17 00:00:00 2001 From: Martine Lenders Date: Wed, 5 Jun 2019 13:03:30 +0200 Subject: [PATCH 3/5] isrpipe: adapt for tsrb API type change --- sys/include/isrpipe.h | 3 ++- sys/isrpipe/isrpipe.c | 4 ++-- sys/isrpipe/read_timeout/read_timeout.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/sys/include/isrpipe.h b/sys/include/isrpipe.h index 356eef82cc..f9bf0460c4 100644 --- a/sys/include/isrpipe.h +++ b/sys/include/isrpipe.h @@ -42,7 +42,8 @@ typedef struct { /** * @brief Static initializer for irspipe */ -#define ISRPIPE_INIT(tsrb_buf) { .mutex = MUTEX_INIT, .tsrb = TSRB_INIT(tsrb_buf) } +#define ISRPIPE_INIT(tsrb_buf) { .mutex = MUTEX_INIT, \ + .tsrb = TSRB_INIT(tsrb_buf) } /** * @brief Initialisation function for isrpipe diff --git a/sys/isrpipe/isrpipe.c b/sys/isrpipe/isrpipe.c index 2a9cf9e2f3..9b8ce170d9 100644 --- a/sys/isrpipe/isrpipe.c +++ b/sys/isrpipe/isrpipe.c @@ -22,7 +22,7 @@ void isrpipe_init(isrpipe_t *isrpipe, char *buf, size_t bufsize) { mutex_init(&isrpipe->mutex); - tsrb_init(&isrpipe->tsrb, buf, bufsize); + tsrb_init(&isrpipe->tsrb, (uint8_t *)buf, bufsize); } int isrpipe_write_one(isrpipe_t *isrpipe, char c) @@ -41,7 +41,7 @@ int isrpipe_read(isrpipe_t *isrpipe, char *buffer, size_t count) { int res; - while (!(res = tsrb_get(&isrpipe->tsrb, buffer, count))) { + while (!(res = tsrb_get(&isrpipe->tsrb, (uint8_t *)buffer, count))) { mutex_lock(&isrpipe->mutex); } return res; diff --git a/sys/isrpipe/read_timeout/read_timeout.c b/sys/isrpipe/read_timeout/read_timeout.c index f2d2e7e2fa..b33a9cbfda 100644 --- a/sys/isrpipe/read_timeout/read_timeout.c +++ b/sys/isrpipe/read_timeout/read_timeout.c @@ -44,7 +44,7 @@ int isrpipe_read_timeout(isrpipe_t *isrpipe, char *buffer, size_t count, uint32_ xtimer_t timer = { .callback = _cb, .arg = &_timeout }; xtimer_set(&timer, timeout); - while (!(res = tsrb_get(&isrpipe->tsrb, buffer, count))) { + while (!(res = tsrb_get(&isrpipe->tsrb, (uint8_t *)buffer, count))) { mutex_lock(&isrpipe->mutex); if (_timeout.flag) { res = -ETIMEDOUT; From 31c49c58f1c9cba0d21e179260b3a00c84d8169f Mon Sep 17 00:00:00 2001 From: Martine Lenders Date: Wed, 5 Jun 2019 13:19:33 +0200 Subject: [PATCH 4/5] ethos: adapt for tsrb API type change --- drivers/ethos/ethos.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/ethos/ethos.c b/drivers/ethos/ethos.c index 6408bc0c95..de636a422a 100644 --- a/drivers/ethos/ethos.c +++ b/drivers/ethos/ethos.c @@ -60,7 +60,7 @@ void ethos_setup(ethos_t *dev, const ethos_params_t *params) dev->last_framesize = 0; dev->accept_new = true; - tsrb_init(&dev->inbuf, (char*)params->buf, params->bufsize); + tsrb_init(&dev->inbuf, params->buf, params->bufsize); mutex_init(&dev->out_mutex); uint32_t a = random_uint32(); @@ -127,7 +127,7 @@ static void _end_of_frame(ethos_t *dev) /* fall through */ case ETHOS_FRAME_TYPE_HELLO_REPLY: if (dev->framesize == 6) { - tsrb_get(&dev->inbuf, (char*)dev->remote_mac_addr, 6); + tsrb_get(&dev->inbuf, dev->remote_mac_addr, 6); } break; } From 5dafce71be434b4bfc1c240518e1b35f688c88ba Mon Sep 17 00:00:00 2001 From: Martine Lenders Date: Wed, 5 Jun 2019 16:24:52 +0200 Subject: [PATCH 5/5] slipdev: adapt for tsrb API type change --- drivers/slipdev/slipdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/slipdev/slipdev.c b/drivers/slipdev/slipdev.c index 7880008e15..821bd46f39 100644 --- a/drivers/slipdev/slipdev.c +++ b/drivers/slipdev/slipdev.c @@ -44,7 +44,7 @@ static int _init(netdev_t *netdev) DEBUG("slipdev: initializing device %p on UART %i with baudrate %" PRIu32 "\n", (void *)dev, dev->config.uart, dev->config.baudrate); /* initialize buffers */ - tsrb_init(&dev->inbuf, dev->rxmem, sizeof(dev->rxmem)); + tsrb_init(&dev->inbuf, (uint8_t *)dev->rxmem, sizeof(dev->rxmem)); if (uart_init(dev->config.uart, dev->config.baudrate, _slip_rx_cb, dev) != UART_OK) { LOG_ERROR("slipdev: error initializing UART %i with baudrate %" PRIu32 "\n",