From cb4e165071ad56c4cf881f5221f02eeefde5de53 Mon Sep 17 00:00:00 2001 From: Kevin O'Connor Date: Thu, 24 May 2018 12:49:23 -0400 Subject: [PATCH] command: Prefer uint8_t* for buffers; prefer uint8_fast_t for lengths Prefer using 'uint8_t' buffers as it is too easy to run into C sign extension problems with 'char' buffers. Prefer using 'uint_fast8_t' for buffer lengths as gcc does a better job compiling them on 32bit mcus. Signed-off-by: Kevin O'Connor --- src/avr/main.c | 2 +- src/avr/usbserial.c | 7 ++--- src/command.c | 61 +++++++++++++++++++-------------------- src/command.h | 15 +++++----- src/generic/crc16_ccitt.c | 2 +- src/generic/misc.h | 2 +- src/generic/serial_irq.c | 15 ++++------ src/generic/usb_cdc.c | 13 ++++----- src/linux/console.c | 8 ++--- src/pru/internal.h | 4 +-- src/pru/pru0.c | 20 ++++++------- 11 files changed, 71 insertions(+), 78 deletions(-) diff --git a/src/avr/main.c b/src/avr/main.c index 2655d307..57aa3c01 100644 --- a/src/avr/main.c +++ b/src/avr/main.c @@ -54,7 +54,7 @@ DECL_INIT(prescaler_init); // Optimized crc16_ccitt for the avr processor uint16_t -crc16_ccitt(char *buf, uint8_t len) +crc16_ccitt(uint8_t *buf, uint_fast8_t len) { uint16_t crc = 0xFFFF; while (len--) diff --git a/src/avr/usbserial.c b/src/avr/usbserial.c index ac60e087..6dbe4f0d 100644 --- a/src/avr/usbserial.c +++ b/src/avr/usbserial.c @@ -10,8 +10,7 @@ #include "command.h" // command_dispatch #include "sched.h" // DECL_INIT -static char receive_buf[MESSAGE_MAX]; -static uint8_t receive_pos; +static uint8_t receive_buf[MESSAGE_MAX], receive_pos; void usbserial_init(void) @@ -51,7 +50,7 @@ void console_task(void) { console_check_input(); - uint8_t pop_count; + uint_fast8_t pop_count; int8_t ret = command_find_block(receive_buf, receive_pos, &pop_count); if (ret > 0) command_dispatch(receive_buf, pop_count); @@ -65,7 +64,7 @@ void console_sendf(const struct command_encoder *ce, va_list args) { // Generate message - static char buf[MESSAGE_MAX]; + static uint8_t buf[MESSAGE_MAX]; uint8_t msglen = command_encodef(buf, ce, args); command_add_frame(buf, msglen); diff --git a/src/command.c b/src/command.c index 1519a428..6161a031 100644 --- a/src/command.c +++ b/src/command.c @@ -21,8 +21,8 @@ static uint8_t next_sequence = MESSAGE_DEST; ****************************************************************/ // Encode an integer as a variable length quantity (vlq) -static char * -encode_int(char *p, uint32_t v) +static uint8_t * +encode_int(uint8_t *p, uint32_t v) { int32_t sv = v; if (sv < (3L<<5) && sv >= -(1L<<5)) goto f4; @@ -39,10 +39,9 @@ f4: *p++ = v & 0x7f; // Parse an integer that was encoded as a "variable length quantity" static uint32_t -parse_int(char **pp) +parse_int(uint8_t **pp) { - char *p = *pp; - uint8_t c = *p++; + uint8_t *p = *pp, c = *p++; uint32_t v = c & 0x7f; if ((c & 0x60) == 0x60) v |= -0x20; @@ -55,16 +54,16 @@ parse_int(char **pp) } // Parse an incoming command into 'args' -char * -command_parsef(char *p, char *maxend +uint8_t * +command_parsef(uint8_t *p, uint8_t *maxend , const struct command_parser *cp, uint32_t *args) { - uint8_t num_params = READP(cp->num_params); + uint_fast8_t num_params = READP(cp->num_params); const uint8_t *param_types = READP(cp->param_types); while (num_params--) { if (p > maxend) goto error; - uint8_t t = READP(*param_types); + uint_fast8_t t = READP(*param_types); param_types++; switch (t) { case PT_uint32: @@ -75,7 +74,7 @@ command_parsef(char *p, char *maxend *args++ = parse_int(&p); break; case PT_buffer: { - uint8_t len = *p++; + uint_fast8_t len = *p++; if (p + len > maxend) goto error; *args++ = len; @@ -93,22 +92,22 @@ error: } // Encode a message -uint8_t -command_encodef(char *buf, const struct command_encoder *ce, va_list args) +uint_fast8_t +command_encodef(uint8_t *buf, const struct command_encoder *ce, va_list args) { - uint8_t max_size = READP(ce->max_size); + uint_fast8_t max_size = READP(ce->max_size); if (max_size <= MESSAGE_MIN) // Ack/Nak message return max_size; - char *p = &buf[MESSAGE_HEADER_SIZE]; - char *maxend = &p[max_size - MESSAGE_MIN]; - uint8_t num_params = READP(ce->num_params); + uint8_t *p = &buf[MESSAGE_HEADER_SIZE]; + uint8_t *maxend = &p[max_size - MESSAGE_MIN]; + uint_fast8_t num_params = READP(ce->num_params); const uint8_t *param_types = READP(ce->param_types); *p++ = READP(ce->msg_id); while (num_params--) { if (p > maxend) goto error; - uint8_t t = READP(*param_types); + uint_fast8_t t = READP(*param_types); param_types++; uint32_t v; switch (t) { @@ -127,7 +126,7 @@ command_encodef(char *buf, const struct command_encoder *ce, va_list args) p = encode_int(p, v); break; case PT_string: { - char *s = va_arg(args, char*), *lenp = p++; + uint8_t *s = va_arg(args, uint8_t*), *lenp = p++; while (*s && p maxend-p) v = maxend-p; *p++ = v; - char *s = va_arg(args, char*); + uint8_t *s = va_arg(args, uint8_t*); if (t == PT_progmem_buffer) memcpy_P(p, s, v); else @@ -158,7 +157,7 @@ error: // Add header and trailer bytes to a message block void -command_add_frame(char *buf, uint8_t msglen) +command_add_frame(uint8_t *buf, uint_fast8_t msglen) { buf[MESSAGE_POS_LEN] = msglen; buf[MESSAGE_POS_SEQ] = next_sequence; @@ -202,7 +201,7 @@ DECL_SHUTDOWN(sendf_shutdown); // Find the command handler associated with a command static const struct command_parser * -command_lookup_parser(uint8_t cmdid) +command_lookup_parser(uint_fast8_t cmdid) { if (!cmdid || cmdid >= READP(command_index_size)) shutdown("Invalid command"); @@ -217,18 +216,18 @@ const struct command_encoder encode_acknak PROGMEM = { enum { CF_NEED_SYNC=1<<0, CF_NEED_VALID=1<<1 }; // Find the next complete message block -int8_t -command_find_block(char *buf, uint8_t buf_len, uint8_t *pop_count) +int_fast8_t +command_find_block(uint8_t *buf, uint_fast8_t buf_len, uint_fast8_t *pop_count) { static uint8_t sync_state; if (buf_len && sync_state & CF_NEED_SYNC) goto need_sync; if (buf_len < MESSAGE_MIN) goto need_more_data; - uint8_t msglen = buf[MESSAGE_POS_LEN]; + uint_fast8_t msglen = buf[MESSAGE_POS_LEN]; if (msglen < MESSAGE_MIN || msglen > MESSAGE_MAX) goto error; - uint8_t msgseq = buf[MESSAGE_POS_SEQ]; + uint_fast8_t msgseq = buf[MESSAGE_POS_SEQ]; if ((msgseq & ~MESSAGE_SEQ_MASK) != MESSAGE_DEST) goto error; if (buf_len < msglen) @@ -236,7 +235,7 @@ command_find_block(char *buf, uint8_t buf_len, uint8_t *pop_count) if (buf[msglen-MESSAGE_TRAILER_SYNC] != MESSAGE_SYNC) goto error; uint16_t msgcrc = ((buf[msglen-MESSAGE_TRAILER_CRC] << 8) - | (uint8_t)buf[msglen-MESSAGE_TRAILER_CRC+1]); + | buf[msglen-MESSAGE_TRAILER_CRC+1]); uint16_t crc = crc16_ccitt(buf, msglen-MESSAGE_TRAILER_SIZE); if (crc != msgcrc) goto error; @@ -263,7 +262,7 @@ error: sync_state |= CF_NEED_SYNC; need_sync: ; // Discard bytes until next SYNC found - char *next_sync = memchr(buf, MESSAGE_SYNC, buf_len); + uint8_t *next_sync = memchr(buf, MESSAGE_SYNC, buf_len); if (next_sync) { sync_state &= ~CF_NEED_SYNC; *pop_count = next_sync - buf + 1; @@ -280,12 +279,12 @@ nak: // Dispatch all the commands found in a message block void -command_dispatch(char *buf, uint8_t msglen) +command_dispatch(uint8_t *buf, uint_fast8_t msglen) { - char *p = &buf[MESSAGE_HEADER_SIZE]; - char *msgend = &buf[msglen-MESSAGE_TRAILER_SIZE]; + uint8_t *p = &buf[MESSAGE_HEADER_SIZE]; + uint8_t *msgend = &buf[msglen-MESSAGE_TRAILER_SIZE]; while (p < msgend) { - uint8_t cmdid = *p++; + uint_fast8_t cmdid = *p++; const struct command_parser *cp = command_lookup_parser(cmdid); uint32_t args[READP(cp->num_args)]; p = command_parsef(p, msgend, cp, args); diff --git a/src/command.h b/src/command.h index e493c3ee..dd960cdf 100644 --- a/src/command.h +++ b/src/command.h @@ -61,14 +61,15 @@ enum { }; // command.c -char *command_parsef(char *p, char *maxend - , const struct command_parser *cp, uint32_t *args); -uint8_t command_encodef(char *buf, const struct command_encoder *ce - , va_list args); +uint8_t *command_parsef(uint8_t *p, uint8_t *maxend + , const struct command_parser *cp, uint32_t *args); +uint_fast8_t command_encodef(uint8_t *buf, const struct command_encoder *ce + , va_list args); void command_sendf(const struct command_encoder *ce, ...); -void command_add_frame(char *buf, uint8_t msglen); -int8_t command_find_block(char *buf, uint8_t buf_len, uint8_t *pop_count); -void command_dispatch(char *buf, uint8_t msglen); +void command_add_frame(uint8_t *buf, uint_fast8_t msglen); +int_fast8_t command_find_block(uint8_t *buf, uint_fast8_t buf_len + , uint_fast8_t *pop_count); +void command_dispatch(uint8_t *buf, uint_fast8_t msglen); // out/compile_time_request.c (auto generated file) extern const struct command_parser command_index[]; diff --git a/src/generic/crc16_ccitt.c b/src/generic/crc16_ccitt.c index 8347bb83..87d08cac 100644 --- a/src/generic/crc16_ccitt.c +++ b/src/generic/crc16_ccitt.c @@ -8,7 +8,7 @@ // Implement the standard crc "ccitt" algorithm on the given buffer uint16_t -crc16_ccitt(char *buf, uint8_t len) +crc16_ccitt(uint8_t *buf, uint_fast8_t len) { uint16_t crc = 0xffff; while (len--) { diff --git a/src/generic/misc.h b/src/generic/misc.h index 65861207..eebd16d8 100644 --- a/src/generic/misc.h +++ b/src/generic/misc.h @@ -16,6 +16,6 @@ void timer_kick(void); void *dynmem_start(void); void *dynmem_end(void); -uint16_t crc16_ccitt(char *buf, uint8_t len); +uint16_t crc16_ccitt(uint8_t *buf, uint_fast8_t len); #endif // misc.h diff --git a/src/generic/serial_irq.c b/src/generic/serial_irq.c index 79eaad7f..7c7842ea 100644 --- a/src/generic/serial_irq.c +++ b/src/generic/serial_irq.c @@ -14,10 +14,8 @@ #include "sched.h" // sched_wake_tasks #include "serial_irq.h" // serial_enable_tx_irq -static char receive_buf[192]; -static uint8_t receive_pos; -static char transmit_buf[96]; -static uint8_t transmit_pos, transmit_max; +static uint8_t receive_buf[192], receive_pos; +static uint8_t transmit_buf[96], transmit_pos, transmit_max; DECL_CONSTANT(SERIAL_BAUD, CONFIG_SERIAL_BAUD); @@ -73,9 +71,8 @@ console_pop_input(uint_fast8_t len) void console_task(void) { - uint_fast8_t rpos = readb(&receive_pos); - uint8_t pop_count; - int8_t ret = command_find_block(receive_buf, rpos, &pop_count); + uint_fast8_t rpos = readb(&receive_pos), pop_count; + int_fast8_t ret = command_find_block(receive_buf, rpos, &pop_count); if (ret > 0) command_dispatch(receive_buf, pop_count); if (ret) @@ -110,8 +107,8 @@ console_sendf(const struct command_encoder *ce, va_list args) } // Generate message - char *buf = &transmit_buf[tmax]; - uint8_t msglen = command_encodef(buf, ce, args); + uint8_t *buf = &transmit_buf[tmax]; + uint_fast8_t msglen = command_encodef(buf, ce, args); command_add_frame(buf, msglen); // Start message transmit diff --git a/src/generic/usb_cdc.c b/src/generic/usb_cdc.c index fea567ec..49bb483e 100644 --- a/src/generic/usb_cdc.c +++ b/src/generic/usb_cdc.c @@ -23,8 +23,7 @@ ****************************************************************/ static struct task_wake usb_bulk_in_wake; -static char transmit_buf[96]; -static uint8_t transmit_pos; +static uint8_t transmit_buf[96], transmit_pos; void usb_notify_bulk_in(void) @@ -65,8 +64,8 @@ console_sendf(const struct command_encoder *ce, va_list args) return; // Generate message - char *buf = &transmit_buf[tpos]; - uint8_t msglen = command_encodef(buf, ce, args); + uint8_t *buf = &transmit_buf[tpos]; + uint_fast8_t msglen = command_encodef(buf, ce, args); command_add_frame(buf, msglen); // Start message transmit @@ -80,8 +79,7 @@ console_sendf(const struct command_encoder *ce, va_list args) ****************************************************************/ static struct task_wake usb_bulk_out_wake; -static char receive_buf[128]; -static uint8_t receive_pos; +static uint8_t receive_buf[128], receive_pos; void usb_notify_bulk_out(void) @@ -95,8 +93,7 @@ usb_bulk_out_task(void) if (!sched_check_wake(&usb_bulk_out_wake)) return; // Process any existing message blocks - uint_fast8_t rpos = receive_pos; - uint8_t pop_count; + uint_fast8_t rpos = receive_pos, pop_count; int_fast8_t ret = command_find_block(receive_buf, rpos, &pop_count); if (ret > 0) command_dispatch(receive_buf, pop_count); diff --git a/src/linux/console.c b/src/linux/console.c index ba6223a0..a1086942 100644 --- a/src/linux/console.c +++ b/src/linux/console.c @@ -128,7 +128,7 @@ console_setup(char *name) ****************************************************************/ static struct task_wake console_wake; -static char receive_buf[4096]; +static uint8_t receive_buf[4096]; static int receive_pos; // Process any incoming commands @@ -155,7 +155,7 @@ console_task(void) // Find and dispatch message blocks in the input int len = receive_pos + ret; - uint8_t pop_count, msglen = len > MESSAGE_MAX ? MESSAGE_MAX : len; + uint_fast8_t pop_count, msglen = len > MESSAGE_MAX ? MESSAGE_MAX : len; ret = command_find_block(receive_buf, msglen, &pop_count); if (ret > 0) command_dispatch(receive_buf, pop_count); @@ -175,8 +175,8 @@ void console_sendf(const struct command_encoder *ce, va_list args) { // Generate message - char buf[MESSAGE_MAX]; - uint8_t msglen = command_encodef(buf, ce, args); + uint8_t buf[MESSAGE_MAX]; + uint_fast8_t msglen = command_encodef(buf, ce, args); command_add_frame(buf, msglen); // Transmit message diff --git a/src/pru/internal.h b/src/pru/internal.h index abbc9e40..dc125cd3 100644 --- a/src/pru/internal.h +++ b/src/pru/internal.h @@ -25,7 +25,7 @@ // Layout of shared memory struct shared_response_buffer { uint32_t count; - char data[MESSAGE_MAX]; + uint8_t data[MESSAGE_MAX]; }; struct shared_mem { uint32_t signal; @@ -36,7 +36,7 @@ struct shared_mem { const struct command_parser *command_index; uint32_t command_index_size; const struct command_parser *shutdown_handler; - char read_data[512]; + uint8_t read_data[512]; }; #define SIGNAL_PRU0_WAITING 0xefefefef diff --git a/src/pru/pru0.c b/src/pru/pru0.c index 58f15175..2a136e9e 100644 --- a/src/pru/pru0.c +++ b/src/pru/pru0.c @@ -32,7 +32,7 @@ static uint16_t transport_dst; #define CHAN_PORT 30 #define RPMSG_HDR_SIZE 16 -static char transmit_buf[RPMSG_BUF_SIZE - RPMSG_HDR_SIZE]; +static uint8_t transmit_buf[RPMSG_BUF_SIZE - RPMSG_HDR_SIZE]; static int transmit_pos; // Transmit all pending message blocks @@ -48,7 +48,7 @@ flush_messages(void) // Generate a message block and queue it for transmission static void -build_message(char *msg, int msglen) +build_message(uint8_t *msg, int msglen) { if (transmit_pos + msglen > sizeof(transmit_buf)) flush_messages(); @@ -105,13 +105,13 @@ send_pru1_shutdown(void) // Dispatch all the commands in a message block static void -do_dispatch(char *buf, uint32_t msglen) +do_dispatch(uint8_t *buf, uint32_t msglen) { - char *p = &buf[MESSAGE_HEADER_SIZE]; - char *msgend = &buf[msglen-MESSAGE_TRAILER_SIZE]; + uint8_t *p = &buf[MESSAGE_HEADER_SIZE]; + uint8_t *msgend = &buf[msglen-MESSAGE_TRAILER_SIZE]; while (p < msgend) { // Parse command - uint8_t cmdid = *p++; + uint_fast8_t cmdid = *p++; const struct command_parser *cp = &SHARED_MEM->command_index[cmdid]; if (!cmdid || cmdid >= SHARED_MEM->command_index_size || cp->num_args > ARRAY_SIZE(SHARED_MEM->next_command_args)) { @@ -131,7 +131,7 @@ check_can_read(void) { // Read data uint16_t dst, len; - char *p = SHARED_MEM->read_data; + uint8_t *p = SHARED_MEM->read_data; int16_t ret = pru_rpmsg_receive(&transport, &transport_dst, &dst, p, &len); if (ret) return ret == PRU_RPMSG_NO_BUF_AVAILABLE; @@ -144,8 +144,8 @@ check_can_read(void) // Parse data into message blocks for (;;) { - uint8_t pop_count, msglen = len > MESSAGE_MAX ? MESSAGE_MAX : len; - int8_t ret = command_find_block(p, msglen, &pop_count); + uint_fast8_t pop_count, msglen = len > MESSAGE_MAX ? MESSAGE_MAX : len; + int_fast8_t ret = command_find_block(p, msglen, &pop_count); if (!ret) break; if (ret > 0) @@ -210,7 +210,7 @@ sched_shutdown(uint_fast8_t reason) void console_sendf(const struct command_encoder *ce, va_list args) { - char buf[MESSAGE_MIN]; + uint8_t buf[MESSAGE_MIN]; build_message(buf, sizeof(buf)); }