I've tripped across several bugs caused by bad printf format strings.
This is foolish, since GCC will tell about them when functions have
proper annotations.
This patch adds annotations to the key command_*() helper functions.
And then fixes the bugs that turned up. Test builds on Linux:
- x86_64 with gcc 4.3.2
- x86_32 with gcc 4.3.3
- armel with gcc 4.3.2
Several of these bugs were from misuse of PRIi64; that's for 64-bit
integers, NOT for "long long" or "u64" (which work best with %lld).
(NOTE that the armel build turned up *LOTS* of unrelated bugs, not fixed
here. Biggest: abusing "u8 *ptr" by "*((u32 *)ptr) = ..." loses badly,
since ARM doesn't guarantee unaligned reads work. That idiom is used
all over the place in JTAG buffer conversions.)
I've tripped across several bugs caused by bad printf format strings.
This is foolish, since GCC will tell about them when functions have
proper annotations.
This patch adds annotations to the key command_*() helper functions.
And then fixes the bugs that turned up. Test builds on:
- x86_64 with gcc 4.3.2
- x86_32 with gcc 4.3.3
- armel with gcc 4.3.2
Several of these bugs were from misuse of PRIi64; that's for 64-bit
integers, NOT for "long long" or "u64" (which work best with %lld).
(NOTE that the armel build turned up *LOTS* of unrelated bugs, not fixed
here. Biggest: using "u8 *ptr" by "*((u32 *)ptr) = ..." loses badly,
since ARM doesn't guarantee unaligned reads work. That idiom is used
all over the place in JTAG buffer conversions.)
---
src/flash/flash.c | 11 +++++++----
src/flash/nand.c | 10 +++++-----
src/helper/command.h | 10 +++++++---
src/pld/pld.c | 9 +++++----
src/svf/svf.c | 2 +-
src/target/armv4_5.c | 2 +-
src/target/armv4_5_mmu.c | 2 +-
src/target/etm.c | 2 +-
src/target/target.c | 8 +++++---
src/target/trace.c | 2 +-
10 files changed, 34 insertions(+), 24 deletions(-)
--- a/src/flash/flash.c
+++ b/src/flash/flash.c
@@ -432,7 +432,7 @@ static int handle_flash_erase_check_comm
int j;
if ((retval = p->driver->erase_check(p)) == ERROR_OK)
{
- command_print(cmd_ctx, "successfully checked erase state", p->driver->name, p->base);
+ command_print(cmd_ctx, "successfully checked erase state");
}
else
{
@@ -567,7 +567,8 @@ static int handle_flash_erase_command(st
return retval;
}
- command_print(cmd_ctx, "erased sectors %i through %i on flash bank %i in %s", first, last, strtoul(args[0], 0, 0), duration_text);
+ command_print(cmd_ctx, "erased sectors %i through %i on flash bank %li in %s",
+ first, last, strtoul(args[0], 0, 0), duration_text);
free(duration_text);
}
}
@@ -606,7 +607,9 @@ static int handle_flash_protect_command(
retval = flash_driver_protect(p, set, first, last);
if (retval == ERROR_OK)
{
- command_print(cmd_ctx, "%s protection for sectors %i through %i on flash bank %i", (set) ? "set" : "cleared", first, last, strtoul(args[0], 0, 0));
+ command_print(cmd_ctx, "%s protection for sectors %i through %i on flash bank %li",
+ (set) ? "set" : "cleared", first,
+ last, strtoul(args[0], 0, 0));
}
}
else
@@ -873,7 +876,7 @@ static int handle_flash_write_bank_comma
}
if (retval==ERROR_OK)
{
- command_print(cmd_ctx, "wrote %"PRIi64" byte from file %s to flash bank %i at offset 0x%8.8x in %s (%f kb/s)",
+ command_print(cmd_ctx, "wrote %lld byte from file %s to flash bank %li at offset 0x%8.8x in %s (%f kb/s)",
fileio.size, args[1], strtoul(args[0], NULL, 0), offset, duration_text,
(float)fileio.size / 1024.0 / ((float)duration.duration.tv_sec + ((float)duration.duration.tv_usec / 1000000.0)));
}
--- a/src/flash/nand.c
+++ b/src/flash/nand.c
@@ -1148,7 +1148,7 @@ static int handle_nand_info_command(stru
}
else
{
- command_print(cmd_ctx, "#%i: not probed");
+ command_print(cmd_ctx, "#%s: not probed", args[0]);
}
}
@@ -1251,7 +1251,7 @@ int handle_nand_check_bad_blocks_command
{
if ((retval = nand_build_bbt(p, first, last)) == ERROR_OK)
{
- command_print(cmd_ctx, "checked NAND flash device for bad blocks, use \"nand info\" command to list blocks", p->device->name);
+ command_print(cmd_ctx, "checked NAND flash device for bad blocks, use \"nand info\" command to list blocks");
}
else if (retval == ERROR_NAND_OPERATION_FAILED)
{
@@ -1570,13 +1570,13 @@ static int handle_nand_dump_command(stru
fileio_close(&fileio);
duration_stop_measure(&duration, &duration_text);
- command_print(cmd_ctx, "dumped %"PRIi64" byte in %s", fileio.size, duration_text);
+ command_print(cmd_ctx, "dumped %lld byte in %s", fileio.size, duration_text);
free(duration_text);
duration_text = NULL;
}
else
{
- command_print(cmd_ctx, "#%i: not probed");
+ command_print(cmd_ctx, "#%s: not probed", args[0]);
}
}
else
@@ -1621,7 +1621,7 @@ static int handle_nand_raw_access_comman
}
else
{
- command_print(cmd_ctx, "#%i: not probed");
+ command_print(cmd_ctx, "#%s: not probed", args[0]);
}
}
else
--- a/src/helper/command.h
+++ b/src/helper/command.h
@@ -83,10 +83,14 @@ extern command_context_t* copy_command_c
extern int command_context_mode(command_context_t *context, enum command_mode mode);
extern command_context_t* command_init(void);
extern int command_done(command_context_t *context);
-extern void command_print(command_context_t *context, char *format, ...);
-extern void command_print_sameline(command_context_t *context, char *format, ...);
+
+extern void command_print(command_context_t *context, char *format, ...)
+ __attribute__ ((format (printf, 2, 3)));
+extern void command_print_sameline(command_context_t *context, char *format, ...)
+ __attribute__ ((format (printf, 2, 3)));
extern int command_run_line(command_context_t *context, char *line);
-extern int command_run_linef(command_context_t *context, char *format, ...);
+extern int command_run_linef(command_context_t *context, char *format, ...)
+ __attribute__ ((format (printf, 2, 3)));
extern void command_output_text(command_context_t *context, const char *data);
extern void process_jim_events(void);
--- a/src/pld/pld.c
+++ b/src/pld/pld.c
@@ -178,7 +178,7 @@ int handle_pld_load_command(struct comma
if ((retval = p->driver->load(p, args[1])) != ERROR_OK)
{
- command_print(cmd_ctx, "failed loading file %s to pld device %i",
+ command_print(cmd_ctx, "failed loading file %s to pld device %lu",
args[1], strtoul(args[0], NULL, 0));
switch (retval)
{
@@ -188,9 +188,10 @@ int handle_pld_load_command(struct comma
{
gettimeofday(&end, NULL);
timeval_subtract(&duration, &end, &start);
-
- command_print(cmd_ctx, "loaded file %s to pld device %i in %is %ius",
- args[1], strtoul(args[0], NULL, 0), duration.tv_sec, duration.tv_usec);
+
+ command_print(cmd_ctx, "loaded file %s to pld device %lu in %lis %lius",
+ args[1], strtoul(args[0], NULL, 0),
+ duration.tv_sec, duration.tv_usec);
}
return ERROR_OK;
--- a/src/svf/svf.c
+++ b/src/svf/svf.c
@@ -330,7 +330,7 @@ static int handle_svf_command(struct com
}
// print time
- command_print(cmd_ctx, "%d ms used", timeval_ms() - time_ago);
+ command_print(cmd_ctx, "%lld ms used", timeval_ms() - time_ago);
free_all:
--- a/src/target/armv4_5.c
+++ b/src/target/armv4_5.c
@@ -342,7 +342,7 @@ int handle_armv4_5_reg_command(struct co
output_len += snprintf(output + output_len, 128 - output_len, "%8s: %8.8x ", ARMV4_5_CORE_REG_MODENUM(armv4_5->core_cache, mode, num).name,
buf_get_u32(ARMV4_5_CORE_REG_MODENUM(armv4_5->core_cache, mode, num).value, 0, 32));
}
- command_print(cmd_ctx, output);
+ command_print(cmd_ctx, "%s", output);
}
command_print(cmd_ctx, " cpsr: %8.8x spsr_fiq: %8.8x spsr_irq: %8.8x spsr_svc: %8.8x spsr_abt: %8.8x spsr_und: %8.8x",
buf_get_u32(armv4_5->core_cache->reg_list[ARMV4_5_CPSR].value, 0, 32),
--- a/src/target/armv4_5_mmu.c
+++ b/src/target/armv4_5_mmu.c
@@ -299,7 +299,7 @@ int armv4_5_mmu_handle_md_phys_command(c
if ((i % 8 == 7) || (i == count - 1))
{
- command_print(cmd_ctx, output);
+ command_print(cmd_ctx, "%s", output);
output_len = 0;
}
}
--- a/src/target/etm.c
+++ b/src/target/etm.c
@@ -1674,7 +1674,7 @@ static int handle_etm_trigger_percent_co
if ((new_value < 2) || (new_value > 100))
{
- command_print(cmd_ctx, "valid settings are 2% to 100%");
+ command_print(cmd_ctx, "valid settings are 2%% to 100%%");
}
else
{
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -1910,7 +1910,7 @@ static int handle_md_command(struct comm
if ((i%line_modulo == line_modulo-1) || (i == count - 1))
{
- command_print(cmd_ctx, output);
+ command_print(cmd_ctx, "%s", output);
output_len = 0;
}
}
@@ -2168,7 +2168,8 @@ static int handle_dump_image_command(str
if (retval==ERROR_OK)
{
- command_print(cmd_ctx, "dumped %"PRIi64" byte in %s", fileio.size, duration_text);
+ command_print(cmd_ctx, "dumped %lld byte in %s",
+ fileio.size, duration_text);
free(duration_text);
}
@@ -2369,7 +2370,8 @@ static int handle_bp_command(struct comm
}
else
{
- command_print(cmd_ctx, "breakpoint added at address 0x%8.8x", strtoul(args[0], NULL, 0));
+ command_print(cmd_ctx, "breakpoint added at address 0x%8.8lx",
+ strtoul(args[0], NULL, 0));
}
}
else
--- a/src/target/trace.c
+++ b/src/target/trace.c
@@ -60,7 +60,7 @@ static int handle_trace_point_command(st
for (i = 0; i < trace->num_trace_points; i++)
{
- command_print(cmd_ctx, "trace point 0x%8.8x (%"PRIi64" times hit)",
+ command_print(cmd_ctx, "trace point 0x%8.8x (%lld times hit)",
trace->trace_points[i].address,
trace->trace_points[i].hit_counter);
}
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
