From 3c1bd502177ddb1af7e1df3b4d11fa0dd3a0fc27 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Thu, 25 Dec 2025 15:53:07 +0100 Subject: [PATCH] target: clean up return value of target_type::blank_check_memory() The functions in struct target_type::blank_check_memory() return either an OpenOCD error or a positive value that indicates the number of blocks checked. To prevent the mix of error codes and returned values, return the value through an additional parameter 'checked' and then return ERROR_OK. While there: - change to unsigned int the parameter 'num_blocks'; - in armv7m_blank_check_memory() verify that the working area can contain at least two 'algo_block'. Change-Id: Ie22f5816819bc77ec611c3f251373d026ed9f784 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/9386 Tested-by: jenkins Reviewed-by: Tomas Vanek --- src/flash/nor/core.c | 7 ++++--- src/flash/nor/psoc5lp.c | 7 ++++--- src/target/arm.h | 3 ++- src/target/armv4_5.c | 12 ++++++------ src/target/armv7m.c | 15 +++++++++++---- src/target/armv7m.h | 3 ++- src/target/mips32.c | 13 ++++++------- src/target/mips32.h | 3 ++- src/target/stm8.c | 12 ++++++------ src/target/target.c | 7 ++++--- src/target/target.h | 4 ++-- src/target/target_type.h | 4 ++-- 12 files changed, 51 insertions(+), 39 deletions(-) diff --git a/src/flash/nor/core.c b/src/flash/nor/core.c index 5c4f2acca..9d24548f4 100644 --- a/src/flash/nor/core.c +++ b/src/flash/nor/core.c @@ -400,17 +400,18 @@ int default_flash_blank_check(struct flash_bank *bank) bool fast_check = true; for (unsigned int i = 0; i < bank->num_sectors; ) { + unsigned int checked; retval = target_blank_check_memory(target, block_array + i, bank->num_sectors - i, - bank->erased_value); - if (retval < 1) { + bank->erased_value, &checked); + if (retval != ERROR_OK) { /* Run slow fallback if the first run gives no result * otherwise use possibly incomplete results */ if (i == 0) fast_check = false; break; } - i += retval; /* add number of blocks done this round */ + i += checked; /* add number of blocks done this round */ } if (fast_check) { diff --git a/src/flash/nor/psoc5lp.c b/src/flash/nor/psoc5lp.c index e8c901950..6b46e3902 100644 --- a/src/flash/nor/psoc5lp.c +++ b/src/flash/nor/psoc5lp.c @@ -1081,17 +1081,18 @@ static int psoc5lp_erase_check(struct flash_bank *bank) bool fast_check = true; for (unsigned int i = 0; i < num_sectors; ) { + unsigned int checked; retval = armv7m_blank_check_memory(target, block_array + i, num_sectors - i, - bank->erased_value); - if (retval < 1) { + bank->erased_value, &checked); + if (retval != ERROR_OK) { /* Run slow fallback if the first run gives no result * otherwise use possibly incomplete results */ if (i == 0) fast_check = false; break; } - i += retval; /* add number of blocks done this round */ + i += checked; /* add number of blocks done this round */ } if (fast_check) { diff --git a/src/target/arm.h b/src/target/arm.h index 54a25731c..6d6412219 100644 --- a/src/target/arm.h +++ b/src/target/arm.h @@ -323,7 +323,8 @@ int armv4_5_run_algorithm_inner(struct target *target, int arm_checksum_memory(struct target *target, target_addr_t address, uint32_t count, uint32_t *checksum); int arm_blank_check_memory(struct target *target, - struct target_memory_check_block *blocks, int num_blocks, uint8_t erased_value); + struct target_memory_check_block *blocks, unsigned int num_blocks, + uint8_t erased_value, unsigned int *checked); void arm_set_cpsr(struct arm *arm, uint32_t cpsr); struct reg *arm_reg_current(struct arm *arm, unsigned int regnum); diff --git a/src/target/armv4_5.c b/src/target/armv4_5.c index 78bcc06ba..4f61c58e2 100644 --- a/src/target/armv4_5.c +++ b/src/target/armv4_5.c @@ -1686,7 +1686,8 @@ int arm_checksum_memory(struct target *target, * */ int arm_blank_check_memory(struct target *target, - struct target_memory_check_block *blocks, int num_blocks, uint8_t erased_value) + struct target_memory_check_block *blocks, unsigned int num_blocks, + uint8_t erased_value, unsigned int *checked) { struct working_area *check_algorithm; struct reg_param reg_params[3]; @@ -1748,8 +1749,10 @@ int arm_blank_check_memory(struct target *target, exit_var, 10000, &arm_algo); - if (retval == ERROR_OK) + if (retval == ERROR_OK) { blocks[0].result = buf_get_u32(reg_params[2].value, 0, 32); + *checked = 1; /* only one block has been checked */ + } destroy_reg_param(®_params[0]); destroy_reg_param(®_params[1]); @@ -1757,10 +1760,7 @@ int arm_blank_check_memory(struct target *target, target_free_working_area(target, check_algorithm); - if (retval != ERROR_OK) - return retval; - - return 1; /* only one block has been checked */ + return retval; } static int arm_full_context(struct target *target) diff --git a/src/target/armv7m.c b/src/target/armv7m.c index b8697c032..07f1f6fc9 100644 --- a/src/target/armv7m.c +++ b/src/target/armv7m.c @@ -973,7 +973,8 @@ cleanup: /** Checks an array of memory regions whether they are erased. */ int armv7m_blank_check_memory(struct target *target, - struct target_memory_check_block *blocks, int num_blocks, uint8_t erased_value) + struct target_memory_check_block *blocks, unsigned int num_blocks, + uint8_t erased_value, unsigned int *checked) { struct working_area *erase_check_algorithm; struct working_area *erase_check_params; @@ -1009,7 +1010,13 @@ int armv7m_blank_check_memory(struct target *target, }; uint32_t avail = target_get_working_area_avail(target); - int blocks_to_check = avail / sizeof(struct algo_block) - 1; + unsigned int avail_blocks = avail / sizeof(struct algo_block); + if (avail_blocks < 2) { + retval = ERROR_TARGET_RESOURCE_NOT_AVAILABLE; + goto cleanup1; + } + + unsigned int blocks_to_check = avail_blocks - 1; if (num_blocks < blocks_to_check) blocks_to_check = num_blocks; @@ -1019,7 +1026,7 @@ int armv7m_blank_check_memory(struct target *target, goto cleanup1; } - int i; + unsigned int i; uint32_t total_size = 0; for (i = 0; i < blocks_to_check; i++) { total_size += blocks[i].size; @@ -1089,7 +1096,7 @@ int armv7m_blank_check_memory(struct target *target, LOG_TARGET_INFO(target, "Slow CPU clock: %d blocks checked, %d remain. Continuing...", i, num_blocks - i); - retval = i; /* return number of blocks really checked */ + *checked = i; /* return number of blocks really checked */ cleanup4: destroy_reg_param(®_params[0]); diff --git a/src/target/armv7m.h b/src/target/armv7m.h index 8020cf034..14c2da460 100644 --- a/src/target/armv7m.h +++ b/src/target/armv7m.h @@ -353,7 +353,8 @@ bool armv7m_map_reg_packing(unsigned int arm_reg_id, int armv7m_checksum_memory(struct target *target, target_addr_t address, uint32_t count, uint32_t *checksum); int armv7m_blank_check_memory(struct target *target, - struct target_memory_check_block *blocks, int num_blocks, uint8_t erased_value); + struct target_memory_check_block *blocks, unsigned int num_blocks, + uint8_t erased_value, unsigned int *checked); int armv7m_maybe_skip_bkpt_inst(struct target *target, bool *inst_found); diff --git a/src/target/mips32.c b/src/target/mips32.c index b8c61a5bf..ce51cc9f8 100644 --- a/src/target/mips32.c +++ b/src/target/mips32.c @@ -1310,8 +1310,8 @@ int mips32_checksum_memory(struct target *target, target_addr_t address, /** Checks whether a memory region is erased. */ int mips32_blank_check_memory(struct target *target, - struct target_memory_check_block *blocks, int num_blocks, - uint8_t erased_value) + struct target_memory_check_block *blocks, unsigned int num_blocks, + uint8_t erased_value, unsigned int *checked) { struct working_area *erase_check_algorithm; struct reg_param reg_params[3]; @@ -1367,8 +1367,10 @@ int mips32_blank_check_memory(struct target *target, retval = target_run_algorithm(target, 0, NULL, 3, reg_params, erase_check_algorithm->address, erase_check_algorithm->address + (sizeof(erase_check_code) - 4), 10000, &mips32_info); - if (retval == ERROR_OK) + if (retval == ERROR_OK) { blocks[0].result = buf_get_u32(reg_params[2].value, 0, 32); + *checked = 1; /* only one block has been checked */ + } destroy_reg_param(®_params[0]); destroy_reg_param(®_params[1]); @@ -1377,10 +1379,7 @@ int mips32_blank_check_memory(struct target *target, cleanup: target_free_working_area(target, erase_check_algorithm); - if (retval != ERROR_OK) - return retval; - - return 1; /* only one block has been checked */ + return retval; } static int mips32_verify_pointer(struct command_invocation *cmd, diff --git a/src/target/mips32.h b/src/target/mips32.h index 3d919e7dd..bd797af51 100644 --- a/src/target/mips32.h +++ b/src/target/mips32.h @@ -933,7 +933,8 @@ int mips32_get_gdb_reg_list(struct target *target, int mips32_checksum_memory(struct target *target, target_addr_t address, uint32_t count, uint32_t *checksum); int mips32_blank_check_memory(struct target *target, - struct target_memory_check_block *blocks, int num_blocks, uint8_t erased_value); + struct target_memory_check_block *blocks, unsigned int num_blocks, + uint8_t erased_value, unsigned int *checked); bool mips32_cpu_support_sync(struct mips_ejtag *ejtag_info); bool mips32_cpu_support_hazard_barrier(struct mips_ejtag *ejtag_info); diff --git a/src/target/stm8.c b/src/target/stm8.c index 05989eeb9..3b5d83ff4 100644 --- a/src/target/stm8.c +++ b/src/target/stm8.c @@ -1757,7 +1757,8 @@ static int stm8_examine(struct target *target) /** Checks whether a memory region is erased. */ static int stm8_blank_check_memory(struct target *target, - struct target_memory_check_block *blocks, int num_blocks, uint8_t erased_value) + struct target_memory_check_block *blocks, unsigned int num_blocks, + uint8_t erased_value, unsigned int *checked) { struct working_area *erase_check_algorithm; struct reg_param reg_params[2]; @@ -1801,8 +1802,10 @@ static int stm8_blank_check_memory(struct target *target, erase_check_algorithm->address + (sizeof(stm8_erase_check_code) - 1), 10000, &stm8_info); - if (retval == ERROR_OK) + if (retval == ERROR_OK) { blocks[0].result = (*(reg_params[0].value) == 0xff); + *checked = 1; /* only one block has been checked */ + } destroy_mem_param(&mem_params[0]); destroy_mem_param(&mem_params[1]); @@ -1811,10 +1814,7 @@ static int stm8_blank_check_memory(struct target *target, target_free_working_area(target, erase_check_algorithm); - if (retval != ERROR_OK) - return retval; - - return 1; /* only one block has been checked */ + return retval; } static int stm8_checksum_memory(struct target *target, target_addr_t address, diff --git a/src/target/target.c b/src/target/target.c index 00fbe239a..f766d1da4 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -2513,8 +2513,8 @@ int target_checksum_memory(struct target *target, target_addr_t address, uint32_ } int target_blank_check_memory(struct target *target, - struct target_memory_check_block *blocks, int num_blocks, - uint8_t erased_value) + struct target_memory_check_block *blocks, unsigned int num_blocks, + uint8_t erased_value, unsigned int *checked) { if (!target_was_examined(target)) { LOG_ERROR("Target not examined yet"); @@ -2524,7 +2524,8 @@ int target_blank_check_memory(struct target *target, if (!target->type->blank_check_memory) return ERROR_NOT_IMPLEMENTED; - return target->type->blank_check_memory(target, blocks, num_blocks, erased_value); + return target->type->blank_check_memory(target, blocks, num_blocks, + erased_value, checked); } int target_read_u64(struct target *target, target_addr_t address, uint64_t *value) diff --git a/src/target/target.h b/src/target/target.h index 4dfc2a595..2b66dd741 100644 --- a/src/target/target.h +++ b/src/target/target.h @@ -662,8 +662,8 @@ int target_read_buffer(struct target *target, int target_checksum_memory(struct target *target, target_addr_t address, uint32_t size, uint32_t *crc); int target_blank_check_memory(struct target *target, - struct target_memory_check_block *blocks, int num_blocks, - uint8_t erased_value); + struct target_memory_check_block *blocks, unsigned int num_blocks, + uint8_t erased_value, unsigned int *checked); int target_wait_state(struct target *target, enum target_state state, unsigned int ms); /** diff --git a/src/target/target_type.h b/src/target/target_type.h index 219417409..68ff2b606 100644 --- a/src/target/target_type.h +++ b/src/target/target_type.h @@ -141,8 +141,8 @@ struct target_type { int (*checksum_memory)(struct target *target, target_addr_t address, uint32_t count, uint32_t *checksum); int (*blank_check_memory)(struct target *target, - struct target_memory_check_block *blocks, int num_blocks, - uint8_t erased_value); + struct target_memory_check_block *blocks, unsigned int num_blocks, + uint8_t erased_value, unsigned int *checked); /* * target break-/watchpoint control