From 355f4cadbbf10c75cf05fb1a82bf918b3ab65c55 Mon Sep 17 00:00:00 2001 From: Paul Fertser Date: Sat, 17 Jan 2015 15:15:11 +0300 Subject: [PATCH] Use (uint8_t *) for buf_(set|get)_u(32|64) instead of (void *) This helps to uncover incorrect usage when a pointer to uint32_t is passed to those functions which leads to subtle bugs on BE systems. The reason is that it's normally assumed that any uint32_t variable holds its value in host byte order, but using but_set_u32 on it silently does implicit pointer conversion to (void *) and the assumption ends up broken without any indication. Change-Id: I48ffd190583d8aa32ec1fef8f1cdc0b4184e4546 Signed-off-by: Paul Fertser Reviewed-on: http://openocd.zylin.com/2467 Tested-by: jenkins Reviewed-by: Andreas Fritiofson --- src/helper/binarybuffer.h | 8 ++++---- src/jtag/drivers/ftdi.c | 2 +- src/target/etb.c | 2 +- src/target/etm.c | 4 ++-- src/target/etm.h | 2 +- src/target/smp.c | 4 ++-- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/helper/binarybuffer.h b/src/helper/binarybuffer.h index 9c20bcd59c..eaa8c52630 100644 --- a/src/helper/binarybuffer.h +++ b/src/helper/binarybuffer.h @@ -39,7 +39,7 @@ * @param num The number of bits from @c value to copy (1-32). * @param value Up to 32 bits that will be copied to _buffer. */ -static inline void buf_set_u32(void *_buffer, +static inline void buf_set_u32(uint8_t *_buffer, unsigned first, unsigned num, uint32_t value) { uint8_t *buffer = _buffer; @@ -68,7 +68,7 @@ static inline void buf_set_u32(void *_buffer, * @param num The number of bits from @c value to copy (1-64). * @param value Up to 64 bits that will be copied to _buffer. */ -static inline void buf_set_u64(void *_buffer, +static inline void buf_set_u64(uint8_t *_buffer, unsigned first, unsigned num, uint64_t value) { uint8_t *buffer = _buffer; @@ -106,7 +106,7 @@ static inline void buf_set_u64(void *_buffer, * @param num The number of bits from @c _buffer to read (1-32). * @returns Up to 32-bits that were read from @c _buffer. */ -static inline uint32_t buf_get_u32(const void *_buffer, +static inline uint32_t buf_get_u32(const uint8_t *_buffer, unsigned first, unsigned num) { const uint8_t *buffer = _buffer; @@ -135,7 +135,7 @@ static inline uint32_t buf_get_u32(const void *_buffer, * @param num The number of bits from @c _buffer to read (1-64). * @returns Up to 64-bits that were read from @c _buffer. */ -static inline uint64_t buf_get_u64(const void *_buffer, +static inline uint64_t buf_get_u64(const uint8_t *_buffer, unsigned first, unsigned num) { const uint8_t *buffer = _buffer; diff --git a/src/jtag/drivers/ftdi.c b/src/jtag/drivers/ftdi.c index c031fd36e8..7df6389e24 100644 --- a/src/jtag/drivers/ftdi.c +++ b/src/jtag/drivers/ftdi.c @@ -965,7 +965,7 @@ static int ftdi_swd_run_queue(struct adiv5_dap *dap) } for (size_t i = 0; i < swd_cmd_queue_length; i++) { - int ack = buf_get_u32(&swd_cmd_queue[i].trn_ack_data_parity_trn, 1, 3); + int ack = buf_get_u32(swd_cmd_queue[i].trn_ack_data_parity_trn, 1, 3); LOG_DEBUG("%s %s %s reg %X = %08"PRIx32, ack == SWD_ACK_OK ? "OK" : ack == SWD_ACK_WAIT ? "WAIT" : ack == SWD_ACK_FAULT ? "FAULT" : "JUNK", diff --git a/src/target/etb.c b/src/target/etb.c index 370c181a99..56f5795bde 100644 --- a/src/target/etb.c +++ b/src/target/etb.c @@ -308,7 +308,7 @@ static int etb_write_reg(struct reg *reg, uint32_t value) fields[0].num_bits = 32; uint8_t temp0[4]; fields[0].out_value = temp0; - buf_set_u32(&temp0, 0, 32, value); + buf_set_u32(temp0, 0, 32, value); fields[0].in_value = NULL; fields[1].num_bits = 7; diff --git a/src/target/etm.c b/src/target/etm.c index 0c27bc3dc6..5239190a53 100644 --- a/src/target/etm.c +++ b/src/target/etm.c @@ -318,7 +318,7 @@ struct reg_cache *etm_build_reg_cache(struct target *target, etm_core, 1); etm_get_reg(reg_list); - etm_ctx->config = buf_get_u32(&arch_info->value, 0, 32); + etm_ctx->config = buf_get_u32(arch_info->value, 0, 32); config = etm_ctx->config; /* figure ETM version then add base registers */ @@ -334,7 +334,7 @@ struct reg_cache *etm_build_reg_cache(struct target *target, etm_core + 1, 1); etm_get_reg(reg_list + 1); etm_ctx->id = buf_get_u32( - &arch_info[1].value, 0, 32); + arch_info[1].value, 0, 32); LOG_DEBUG("ETM ID: %08x", (unsigned) etm_ctx->id); bcd_vers = 0x10 + (((etm_ctx->id) >> 4) & 0xff); diff --git a/src/target/etm.h b/src/target/etm.h index ded18e9754..ff7925bf78 100644 --- a/src/target/etm.h +++ b/src/target/etm.h @@ -72,7 +72,7 @@ enum { }; struct etm_reg { - uint32_t value; + uint8_t value[4]; const struct etm_reg_info *reg_info; struct arm_jtag *jtag_info; }; diff --git a/src/target/smp.c b/src/target/smp.c index e688304a0e..da9ee8b468 100644 --- a/src/target/smp.c +++ b/src/target/smp.c @@ -64,9 +64,9 @@ int gdb_read_smp_packet(struct connection *connection, if (strncmp(packet, "jc", 2) == 0) { const uint32_t len = sizeof(target->gdb_service->core[0]); char hex_buffer[len * 2 + 1]; - char buffer[len]; + uint8_t buffer[len]; buf_set_u32(buffer, 0, len * 8, target->gdb_service->core[0]); - int pkt_len = hexify(hex_buffer, buffer, sizeof(buffer), sizeof(hex_buffer)); + int pkt_len = hexify(hex_buffer, (char *)buffer, sizeof(buffer), sizeof(hex_buffer)); retval = gdb_put_packet(connection, hex_buffer, pkt_len); } -- 2.30.2