From 396e7a9e7383f68638b9bc336b97a558a0308eee Mon Sep 17 00:00:00 2001 From: Nathael Pajani Date: Thu, 26 May 2022 23:26:00 +0200 Subject: [PATCH] Fix part ID handling on device identification. (confusion between negative return code and part ID when unsigned part ID converts to negative signed value). Bug found by Dan Schultz --- isp_commands.c | 9 ++++----- isp_commands.h | 2 +- lpcisp.c | 3 ++- lpcprog.c | 21 +++++++++++---------- prog_commands.c | 3 ++- 5 files changed, 20 insertions(+), 18 deletions(-) diff --git a/isp_commands.c b/isp_commands.c index 5168f36..4adcd61 100644 --- a/isp_commands.c +++ b/isp_commands.c @@ -237,11 +237,10 @@ int isp_cmd_read_uid(void) return 0; } -int isp_cmd_part_id(int quiet) +int isp_cmd_part_id(unsigned long int* part_id, int quiet) { char buf[REP_BUFSIZE]; int ret = 0, len = 0; - unsigned long int part_id = 0; ret = isp_send_cmd_no_args("read-part-id", READ_PART_ID, quiet); if (ret != 0) { @@ -256,12 +255,12 @@ int isp_cmd_part_id(int quiet) return -2; } /* FIXME : some part IDs are on two 32bits values */ - part_id = strtoul(buf, NULL, 10); + *part_id = strtoul(buf, NULL, 10); if (quiet == 0) { - printf("Part ID is 0x%08lx\n", part_id); + printf("Part ID is 0x%08lx\n", *part_id); } - return part_id; + return 0; } int isp_cmd_boot_version(void) diff --git a/isp_commands.h b/isp_commands.h index 42e3224..403e5f8 100644 --- a/isp_commands.h +++ b/isp_commands.h @@ -67,7 +67,7 @@ int isp_cmd_unlock(int quiet); int isp_cmd_read_uid(void); -int isp_cmd_part_id(int quiet); +int isp_cmd_part_id(unsigned long int* part_id, int quiet); int isp_cmd_boot_version(void); diff --git a/lpcisp.c b/lpcisp.c index 8dae594..65e3cc0 100644 --- a/lpcisp.c +++ b/lpcisp.c @@ -288,6 +288,7 @@ int isp_handle_command(char* cmd, int arg_count, char** args) int cmd_found = -1; int ret = 0; int index = 0; + unsigned long int id = 0; if (cmd == NULL) { printf("isp_handle_command called with no command !\n"); @@ -314,7 +315,7 @@ int isp_handle_command(char* cmd, int arg_count, char** args) ret = isp_cmd_unlock(0); break; case 8: /* read-part-id */ - ret = isp_cmd_part_id(0); + ret = isp_cmd_part_id(&id, 0); break; case 9: /* read-boot-version */ ret = isp_cmd_boot_version(); diff --git a/lpcprog.c b/lpcprog.c index a85158d..025dfad 100644 --- a/lpcprog.c +++ b/lpcprog.c @@ -87,20 +87,21 @@ char* parts_file_name = NULL; #define DEFAULT_PART_FILE_NAME_ETC "/etc/lpctools_parts.def" #define DEFAULT_PART_FILE_NAME_CURRENT "./lpctools_parts.def" -static int prog_connect_and_id(int freq); -static int prog_handle_command(char* cmd, int dev_id, int arg_count, char** args); +static int prog_connect_and_id(unsigned long int* id, int freq); +static int prog_handle_command(char* cmd, unsigned long int dev_id, int arg_count, char** args); int main(int argc, char** argv) { int baudrate = SERIAL_BAUD; int crystal_freq = 10000; char* isp_serial_device = NULL; - int dev_id = 0; + unsigned long int dev_id = 0; /* For "command" handling */ char* command = NULL; char** cmd_args = NULL; int nb_cmd_args = 0; + int ret = 0; /* parameter parsing */ @@ -232,8 +233,8 @@ int main(int argc, char** argv) } /* First : sync with device */ - dev_id = prog_connect_and_id(crystal_freq); - if (dev_id < 0) { + ret = prog_connect_and_id(&dev_id, crystal_freq); + if (ret < 0) { printf("Unable to connect to target, consider hard reset of target or link\n"); return -1; } @@ -280,7 +281,7 @@ static struct prog_command prog_cmds_list[] = { * Try to connect to the target and identify the device. * First try sync, and if it fails, try to read id twice. if both fail, then we are not connected */ -static int prog_connect_and_id(int freq) +static int prog_connect_and_id(unsigned long int* id, int freq) { int sync_ret = 0; @@ -290,13 +291,13 @@ static int prog_connect_and_id(int freq) if (sync_ret < 0) { /* If already synchronized, then sync command and the next command fail. Sync failed, maybe we are already sync'ed, then send one command for nothing */ - isp_cmd_part_id(1); + isp_cmd_part_id(id, 1); } - return isp_cmd_part_id(1); + return isp_cmd_part_id(id, 1); } -static int prog_handle_command(char* cmd, int dev_id, int arg_count, char** args) +static int prog_handle_command(char* cmd, unsigned long int dev_id, int arg_count, char** args) { int cmd_found = -1; int ret = 0; @@ -310,7 +311,7 @@ static int prog_handle_command(char* cmd, int dev_id, int arg_count, char** args part = find_part_in_file(dev_id, parts_file_name); if (part == NULL) { - printf("Unknown part number : 0x%08x.\n", dev_id); + printf("Unknown part number : 0x%08lx.\n", dev_id); return -2; } diff --git a/prog_commands.c b/prog_commands.c index 01572c7..bcc160a 100644 --- a/prog_commands.c +++ b/prog_commands.c @@ -45,8 +45,9 @@ extern int trace_on; int get_ids(void) { int ret = 0; + unsigned long int id = 0; - isp_cmd_part_id(0); + isp_cmd_part_id(&id, 0); isp_cmd_read_uid(); isp_cmd_boot_version(); -- 2.43.0