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 <kevin@koconnor.net>
This commit is contained in:
Kevin O'Connor 2018-05-24 12:49:23 -04:00
parent 2a55741ea8
commit cb4e165071
11 changed files with 71 additions and 78 deletions

View File

@ -54,7 +54,7 @@ DECL_INIT(prescaler_init);
// Optimized crc16_ccitt for the avr processor // Optimized crc16_ccitt for the avr processor
uint16_t uint16_t
crc16_ccitt(char *buf, uint8_t len) crc16_ccitt(uint8_t *buf, uint_fast8_t len)
{ {
uint16_t crc = 0xFFFF; uint16_t crc = 0xFFFF;
while (len--) while (len--)

View File

@ -10,8 +10,7 @@
#include "command.h" // command_dispatch #include "command.h" // command_dispatch
#include "sched.h" // DECL_INIT #include "sched.h" // DECL_INIT
static char receive_buf[MESSAGE_MAX]; static uint8_t receive_buf[MESSAGE_MAX], receive_pos;
static uint8_t receive_pos;
void void
usbserial_init(void) usbserial_init(void)
@ -51,7 +50,7 @@ void
console_task(void) console_task(void)
{ {
console_check_input(); console_check_input();
uint8_t pop_count; uint_fast8_t pop_count;
int8_t ret = command_find_block(receive_buf, receive_pos, &pop_count); int8_t ret = command_find_block(receive_buf, receive_pos, &pop_count);
if (ret > 0) if (ret > 0)
command_dispatch(receive_buf, pop_count); command_dispatch(receive_buf, pop_count);
@ -65,7 +64,7 @@ void
console_sendf(const struct command_encoder *ce, va_list args) console_sendf(const struct command_encoder *ce, va_list args)
{ {
// Generate message // Generate message
static char buf[MESSAGE_MAX]; static uint8_t buf[MESSAGE_MAX];
uint8_t msglen = command_encodef(buf, ce, args); uint8_t msglen = command_encodef(buf, ce, args);
command_add_frame(buf, msglen); command_add_frame(buf, msglen);

View File

@ -21,8 +21,8 @@ static uint8_t next_sequence = MESSAGE_DEST;
****************************************************************/ ****************************************************************/
// Encode an integer as a variable length quantity (vlq) // Encode an integer as a variable length quantity (vlq)
static char * static uint8_t *
encode_int(char *p, uint32_t v) encode_int(uint8_t *p, uint32_t v)
{ {
int32_t sv = v; int32_t sv = v;
if (sv < (3L<<5) && sv >= -(1L<<5)) goto f4; 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" // Parse an integer that was encoded as a "variable length quantity"
static uint32_t static uint32_t
parse_int(char **pp) parse_int(uint8_t **pp)
{ {
char *p = *pp; uint8_t *p = *pp, c = *p++;
uint8_t c = *p++;
uint32_t v = c & 0x7f; uint32_t v = c & 0x7f;
if ((c & 0x60) == 0x60) if ((c & 0x60) == 0x60)
v |= -0x20; v |= -0x20;
@ -55,16 +54,16 @@ parse_int(char **pp)
} }
// Parse an incoming command into 'args' // Parse an incoming command into 'args'
char * uint8_t *
command_parsef(char *p, char *maxend command_parsef(uint8_t *p, uint8_t *maxend
, const struct command_parser *cp, uint32_t *args) , 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); const uint8_t *param_types = READP(cp->param_types);
while (num_params--) { while (num_params--) {
if (p > maxend) if (p > maxend)
goto error; goto error;
uint8_t t = READP(*param_types); uint_fast8_t t = READP(*param_types);
param_types++; param_types++;
switch (t) { switch (t) {
case PT_uint32: case PT_uint32:
@ -75,7 +74,7 @@ command_parsef(char *p, char *maxend
*args++ = parse_int(&p); *args++ = parse_int(&p);
break; break;
case PT_buffer: { case PT_buffer: {
uint8_t len = *p++; uint_fast8_t len = *p++;
if (p + len > maxend) if (p + len > maxend)
goto error; goto error;
*args++ = len; *args++ = len;
@ -93,22 +92,22 @@ error:
} }
// Encode a message // Encode a message
uint8_t uint_fast8_t
command_encodef(char *buf, const struct command_encoder *ce, va_list args) 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) if (max_size <= MESSAGE_MIN)
// Ack/Nak message // Ack/Nak message
return max_size; return max_size;
char *p = &buf[MESSAGE_HEADER_SIZE]; uint8_t *p = &buf[MESSAGE_HEADER_SIZE];
char *maxend = &p[max_size - MESSAGE_MIN]; uint8_t *maxend = &p[max_size - MESSAGE_MIN];
uint8_t num_params = READP(ce->num_params); uint_fast8_t num_params = READP(ce->num_params);
const uint8_t *param_types = READP(ce->param_types); const uint8_t *param_types = READP(ce->param_types);
*p++ = READP(ce->msg_id); *p++ = READP(ce->msg_id);
while (num_params--) { while (num_params--) {
if (p > maxend) if (p > maxend)
goto error; goto error;
uint8_t t = READP(*param_types); uint_fast8_t t = READP(*param_types);
param_types++; param_types++;
uint32_t v; uint32_t v;
switch (t) { switch (t) {
@ -127,7 +126,7 @@ command_encodef(char *buf, const struct command_encoder *ce, va_list args)
p = encode_int(p, v); p = encode_int(p, v);
break; break;
case PT_string: { 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) while (*s && p<maxend)
*p++ = *s++; *p++ = *s++;
*lenp = p-lenp-1; *lenp = p-lenp-1;
@ -139,7 +138,7 @@ command_encodef(char *buf, const struct command_encoder *ce, va_list args)
if (v > maxend-p) if (v > maxend-p)
v = maxend-p; v = maxend-p;
*p++ = v; *p++ = v;
char *s = va_arg(args, char*); uint8_t *s = va_arg(args, uint8_t*);
if (t == PT_progmem_buffer) if (t == PT_progmem_buffer)
memcpy_P(p, s, v); memcpy_P(p, s, v);
else else
@ -158,7 +157,7 @@ error:
// Add header and trailer bytes to a message block // Add header and trailer bytes to a message block
void 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_LEN] = msglen;
buf[MESSAGE_POS_SEQ] = next_sequence; buf[MESSAGE_POS_SEQ] = next_sequence;
@ -202,7 +201,7 @@ DECL_SHUTDOWN(sendf_shutdown);
// Find the command handler associated with a command // Find the command handler associated with a command
static const struct command_parser * 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)) if (!cmdid || cmdid >= READP(command_index_size))
shutdown("Invalid command"); 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 }; enum { CF_NEED_SYNC=1<<0, CF_NEED_VALID=1<<1 };
// Find the next complete message block // Find the next complete message block
int8_t int_fast8_t
command_find_block(char *buf, uint8_t buf_len, uint8_t *pop_count) command_find_block(uint8_t *buf, uint_fast8_t buf_len, uint_fast8_t *pop_count)
{ {
static uint8_t sync_state; static uint8_t sync_state;
if (buf_len && sync_state & CF_NEED_SYNC) if (buf_len && sync_state & CF_NEED_SYNC)
goto need_sync; goto need_sync;
if (buf_len < MESSAGE_MIN) if (buf_len < MESSAGE_MIN)
goto need_more_data; 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) if (msglen < MESSAGE_MIN || msglen > MESSAGE_MAX)
goto error; goto error;
uint8_t msgseq = buf[MESSAGE_POS_SEQ]; uint_fast8_t msgseq = buf[MESSAGE_POS_SEQ];
if ((msgseq & ~MESSAGE_SEQ_MASK) != MESSAGE_DEST) if ((msgseq & ~MESSAGE_SEQ_MASK) != MESSAGE_DEST)
goto error; goto error;
if (buf_len < msglen) 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) if (buf[msglen-MESSAGE_TRAILER_SYNC] != MESSAGE_SYNC)
goto error; goto error;
uint16_t msgcrc = ((buf[msglen-MESSAGE_TRAILER_CRC] << 8) 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); uint16_t crc = crc16_ccitt(buf, msglen-MESSAGE_TRAILER_SIZE);
if (crc != msgcrc) if (crc != msgcrc)
goto error; goto error;
@ -263,7 +262,7 @@ error:
sync_state |= CF_NEED_SYNC; sync_state |= CF_NEED_SYNC;
need_sync: ; need_sync: ;
// Discard bytes until next SYNC found // 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) { if (next_sync) {
sync_state &= ~CF_NEED_SYNC; sync_state &= ~CF_NEED_SYNC;
*pop_count = next_sync - buf + 1; *pop_count = next_sync - buf + 1;
@ -280,12 +279,12 @@ nak:
// Dispatch all the commands found in a message block // Dispatch all the commands found in a message block
void void
command_dispatch(char *buf, uint8_t msglen) command_dispatch(uint8_t *buf, uint_fast8_t msglen)
{ {
char *p = &buf[MESSAGE_HEADER_SIZE]; uint8_t *p = &buf[MESSAGE_HEADER_SIZE];
char *msgend = &buf[msglen-MESSAGE_TRAILER_SIZE]; uint8_t *msgend = &buf[msglen-MESSAGE_TRAILER_SIZE];
while (p < msgend) { while (p < msgend) {
uint8_t cmdid = *p++; uint_fast8_t cmdid = *p++;
const struct command_parser *cp = command_lookup_parser(cmdid); const struct command_parser *cp = command_lookup_parser(cmdid);
uint32_t args[READP(cp->num_args)]; uint32_t args[READP(cp->num_args)];
p = command_parsef(p, msgend, cp, args); p = command_parsef(p, msgend, cp, args);

View File

@ -61,14 +61,15 @@ enum {
}; };
// command.c // command.c
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); , const struct command_parser *cp, uint32_t *args);
uint8_t command_encodef(char *buf, const struct command_encoder *ce uint_fast8_t command_encodef(uint8_t *buf, const struct command_encoder *ce
, va_list args); , va_list args);
void command_sendf(const struct command_encoder *ce, ...); void command_sendf(const struct command_encoder *ce, ...);
void command_add_frame(char *buf, uint8_t msglen); void command_add_frame(uint8_t *buf, uint_fast8_t msglen);
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
void command_dispatch(char *buf, uint8_t msglen); , uint_fast8_t *pop_count);
void command_dispatch(uint8_t *buf, uint_fast8_t msglen);
// out/compile_time_request.c (auto generated file) // out/compile_time_request.c (auto generated file)
extern const struct command_parser command_index[]; extern const struct command_parser command_index[];

View File

@ -8,7 +8,7 @@
// Implement the standard crc "ccitt" algorithm on the given buffer // Implement the standard crc "ccitt" algorithm on the given buffer
uint16_t uint16_t
crc16_ccitt(char *buf, uint8_t len) crc16_ccitt(uint8_t *buf, uint_fast8_t len)
{ {
uint16_t crc = 0xffff; uint16_t crc = 0xffff;
while (len--) { while (len--) {

View File

@ -16,6 +16,6 @@ void timer_kick(void);
void *dynmem_start(void); void *dynmem_start(void);
void *dynmem_end(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 #endif // misc.h

View File

@ -14,10 +14,8 @@
#include "sched.h" // sched_wake_tasks #include "sched.h" // sched_wake_tasks
#include "serial_irq.h" // serial_enable_tx_irq #include "serial_irq.h" // serial_enable_tx_irq
static char receive_buf[192]; static uint8_t receive_buf[192], receive_pos;
static uint8_t receive_pos; static uint8_t transmit_buf[96], transmit_pos, transmit_max;
static char transmit_buf[96];
static uint8_t transmit_pos, transmit_max;
DECL_CONSTANT(SERIAL_BAUD, CONFIG_SERIAL_BAUD); DECL_CONSTANT(SERIAL_BAUD, CONFIG_SERIAL_BAUD);
@ -73,9 +71,8 @@ console_pop_input(uint_fast8_t len)
void void
console_task(void) console_task(void)
{ {
uint_fast8_t rpos = readb(&receive_pos); uint_fast8_t rpos = readb(&receive_pos), pop_count;
uint8_t pop_count; int_fast8_t ret = command_find_block(receive_buf, rpos, &pop_count);
int8_t ret = command_find_block(receive_buf, rpos, &pop_count);
if (ret > 0) if (ret > 0)
command_dispatch(receive_buf, pop_count); command_dispatch(receive_buf, pop_count);
if (ret) if (ret)
@ -110,8 +107,8 @@ console_sendf(const struct command_encoder *ce, va_list args)
} }
// Generate message // Generate message
char *buf = &transmit_buf[tmax]; uint8_t *buf = &transmit_buf[tmax];
uint8_t msglen = command_encodef(buf, ce, args); uint_fast8_t msglen = command_encodef(buf, ce, args);
command_add_frame(buf, msglen); command_add_frame(buf, msglen);
// Start message transmit // Start message transmit

View File

@ -23,8 +23,7 @@
****************************************************************/ ****************************************************************/
static struct task_wake usb_bulk_in_wake; static struct task_wake usb_bulk_in_wake;
static char transmit_buf[96]; static uint8_t transmit_buf[96], transmit_pos;
static uint8_t transmit_pos;
void void
usb_notify_bulk_in(void) usb_notify_bulk_in(void)
@ -65,8 +64,8 @@ console_sendf(const struct command_encoder *ce, va_list args)
return; return;
// Generate message // Generate message
char *buf = &transmit_buf[tpos]; uint8_t *buf = &transmit_buf[tpos];
uint8_t msglen = command_encodef(buf, ce, args); uint_fast8_t msglen = command_encodef(buf, ce, args);
command_add_frame(buf, msglen); command_add_frame(buf, msglen);
// Start message transmit // 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 struct task_wake usb_bulk_out_wake;
static char receive_buf[128]; static uint8_t receive_buf[128], receive_pos;
static uint8_t receive_pos;
void void
usb_notify_bulk_out(void) usb_notify_bulk_out(void)
@ -95,8 +93,7 @@ usb_bulk_out_task(void)
if (!sched_check_wake(&usb_bulk_out_wake)) if (!sched_check_wake(&usb_bulk_out_wake))
return; return;
// Process any existing message blocks // Process any existing message blocks
uint_fast8_t rpos = receive_pos; uint_fast8_t rpos = receive_pos, pop_count;
uint8_t pop_count;
int_fast8_t ret = command_find_block(receive_buf, rpos, &pop_count); int_fast8_t ret = command_find_block(receive_buf, rpos, &pop_count);
if (ret > 0) if (ret > 0)
command_dispatch(receive_buf, pop_count); command_dispatch(receive_buf, pop_count);

View File

@ -128,7 +128,7 @@ console_setup(char *name)
****************************************************************/ ****************************************************************/
static struct task_wake console_wake; static struct task_wake console_wake;
static char receive_buf[4096]; static uint8_t receive_buf[4096];
static int receive_pos; static int receive_pos;
// Process any incoming commands // Process any incoming commands
@ -155,7 +155,7 @@ console_task(void)
// Find and dispatch message blocks in the input // Find and dispatch message blocks in the input
int len = receive_pos + ret; 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); ret = command_find_block(receive_buf, msglen, &pop_count);
if (ret > 0) if (ret > 0)
command_dispatch(receive_buf, pop_count); command_dispatch(receive_buf, pop_count);
@ -175,8 +175,8 @@ void
console_sendf(const struct command_encoder *ce, va_list args) console_sendf(const struct command_encoder *ce, va_list args)
{ {
// Generate message // Generate message
char buf[MESSAGE_MAX]; uint8_t buf[MESSAGE_MAX];
uint8_t msglen = command_encodef(buf, ce, args); uint_fast8_t msglen = command_encodef(buf, ce, args);
command_add_frame(buf, msglen); command_add_frame(buf, msglen);
// Transmit message // Transmit message

View File

@ -25,7 +25,7 @@
// Layout of shared memory // Layout of shared memory
struct shared_response_buffer { struct shared_response_buffer {
uint32_t count; uint32_t count;
char data[MESSAGE_MAX]; uint8_t data[MESSAGE_MAX];
}; };
struct shared_mem { struct shared_mem {
uint32_t signal; uint32_t signal;
@ -36,7 +36,7 @@ struct shared_mem {
const struct command_parser *command_index; const struct command_parser *command_index;
uint32_t command_index_size; uint32_t command_index_size;
const struct command_parser *shutdown_handler; const struct command_parser *shutdown_handler;
char read_data[512]; uint8_t read_data[512];
}; };
#define SIGNAL_PRU0_WAITING 0xefefefef #define SIGNAL_PRU0_WAITING 0xefefefef

View File

@ -32,7 +32,7 @@ static uint16_t transport_dst;
#define CHAN_PORT 30 #define CHAN_PORT 30
#define RPMSG_HDR_SIZE 16 #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; static int transmit_pos;
// Transmit all pending message blocks // Transmit all pending message blocks
@ -48,7 +48,7 @@ flush_messages(void)
// Generate a message block and queue it for transmission // Generate a message block and queue it for transmission
static void static void
build_message(char *msg, int msglen) build_message(uint8_t *msg, int msglen)
{ {
if (transmit_pos + msglen > sizeof(transmit_buf)) if (transmit_pos + msglen > sizeof(transmit_buf))
flush_messages(); flush_messages();
@ -105,13 +105,13 @@ send_pru1_shutdown(void)
// Dispatch all the commands in a message block // Dispatch all the commands in a message block
static void static void
do_dispatch(char *buf, uint32_t msglen) do_dispatch(uint8_t *buf, uint32_t msglen)
{ {
char *p = &buf[MESSAGE_HEADER_SIZE]; uint8_t *p = &buf[MESSAGE_HEADER_SIZE];
char *msgend = &buf[msglen-MESSAGE_TRAILER_SIZE]; uint8_t *msgend = &buf[msglen-MESSAGE_TRAILER_SIZE];
while (p < msgend) { while (p < msgend) {
// Parse command // Parse command
uint8_t cmdid = *p++; uint_fast8_t cmdid = *p++;
const struct command_parser *cp = &SHARED_MEM->command_index[cmdid]; const struct command_parser *cp = &SHARED_MEM->command_index[cmdid];
if (!cmdid || cmdid >= SHARED_MEM->command_index_size if (!cmdid || cmdid >= SHARED_MEM->command_index_size
|| cp->num_args > ARRAY_SIZE(SHARED_MEM->next_command_args)) { || cp->num_args > ARRAY_SIZE(SHARED_MEM->next_command_args)) {
@ -131,7 +131,7 @@ check_can_read(void)
{ {
// Read data // Read data
uint16_t dst, len; 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); int16_t ret = pru_rpmsg_receive(&transport, &transport_dst, &dst, p, &len);
if (ret) if (ret)
return ret == PRU_RPMSG_NO_BUF_AVAILABLE; return ret == PRU_RPMSG_NO_BUF_AVAILABLE;
@ -144,8 +144,8 @@ check_can_read(void)
// Parse data into message blocks // Parse data into message blocks
for (;;) { for (;;) {
uint8_t pop_count, msglen = len > MESSAGE_MAX ? MESSAGE_MAX : len; uint_fast8_t pop_count, msglen = len > MESSAGE_MAX ? MESSAGE_MAX : len;
int8_t ret = command_find_block(p, msglen, &pop_count); int_fast8_t ret = command_find_block(p, msglen, &pop_count);
if (!ret) if (!ret)
break; break;
if (ret > 0) if (ret > 0)
@ -210,7 +210,7 @@ sched_shutdown(uint_fast8_t reason)
void void
console_sendf(const struct command_encoder *ce, va_list args) console_sendf(const struct command_encoder *ce, va_list args)
{ {
char buf[MESSAGE_MIN]; uint8_t buf[MESSAGE_MIN];
build_message(buf, sizeof(buf)); build_message(buf, sizeof(buf));
} }