From cb4e165071ad56c4cf881f5221f02eeefde5de53 Mon Sep 17 00:00:00 2001 From: Kevin O'Connor Date: Thu, 24 May 2018 12:49:23 -0400 Subject: 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/command.c | 61 +++++++++++++++++++++++++++++------------------------------ 1 file changed, 30 insertions(+), 31 deletions(-) (limited to 'src/command.c') 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); -- cgit v1.2.3-70-g09d2