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 <fercerpav@gmail.com>
Reviewed-on: http://openocd.zylin.com/2467
Tested-by: jenkins
Reviewed-by: Andreas Fritiofson <andreas.fritiofson@gmail.com>

diff --git a/src/helper/binarybuffer.h b/src/helper/binarybuffer.h
index 9c20bcd59c8802a6599ebb52f11a7f6857c071a7..eaa8c526308eb0a9b2a2a7283ca12b5eed8111d7 100644 (file)
--- 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 c031fd36e8bdd2c853728ef5c300d8bb0127666d..7df6389e24b6c4ea73d2bd34cc8701d6977128f0 100644 (file)
--- 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 370c181a99c8be0c6aff84326d2f33dd85c776e8..56f5795bde003a9d2e4f62de0b07039c6d0c327e 100644 (file)
--- 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 0c27bc3dc6efbe46876608bb13f4c7cdd469c6eb..5239190a535e6b737130c1c1a1878b239cb3522e 100644 (file)
--- 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 ded18e97547e169a598d2631a97c97ba897ae9d6..ff7925bf78ad4dd799ec6277bfaafd5c5be01c7c 100644 (file)
--- 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 e688304a0e25ad2591c8f690b347560075dbb44d..da9ee8b468fa1bf7626260d58eab869a3345d400 100644 (file)
--- 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);
                }