diff --git a/src/helper/binarybuffer.c b/src/helper/binarybuffer.c index 5f38b43ae..3e09143c6 100644 --- a/src/helper/binarybuffer.c +++ b/src/helper/binarybuffer.c @@ -102,7 +102,6 @@ bool buf_cmp_mask(const void *_buf1, const void *_buf2, return buf_cmp_trailing(buf1[last], buf2[last], mask[last], trailing); } - void *buf_set_ones(void *_buf, unsigned size) { uint8_t *buf = _buf; @@ -206,36 +205,75 @@ char *buf_to_hex_str(const void *_buf, unsigned buf_len) return str; } -/** identify radix, and skip radix-prefix (0, 0x or 0X) */ -static void str_radix_guess(const char **_str, unsigned *_str_len, - unsigned *_radix) +static bool str_has_hex_prefix(const char *s) { - unsigned radix = *_radix; - if (radix != 0) - return; - const char *str = *_str; - unsigned str_len = *_str_len; - if (str[0] == '0' && (str[1] == 'x' || str[1] == 'X')) { - radix = 16; - str += 2; - str_len -= 2; - } else if ((str[0] == '0') && (str_len != 1)) { - radix = 8; - str += 1; - str_len -= 1; - } else - radix = 10; - *_str = str; - *_str_len = str_len; - *_radix = radix; + /* Starts with "0x" or "0X" */ + return (s[0] == '0') && (s[1] == 'x' || s[1] == 'X'); } -int str_to_buf(const char *str, unsigned str_len, - void *_buf, unsigned buf_len, unsigned radix) +static bool str_has_octal_prefix(const char *s) { - str_radix_guess(&str, &str_len, &radix); + /* - starts with '0', + * - has at least two characters, and + * - the second character is not 'x' or 'X' */ + return (s[0] == '0') && (s[1] != '\0') && (s[1] != 'x') && (s[1] != 'X'); +} - float factor; +/** + * Try to identify the radix of the number by looking at its prefix. + * No further validation of the number is preformed. + */ +static unsigned int str_radix_guess(const char *str) +{ + assert(str); + + if (str_has_hex_prefix(str)) + return 16; + + if (str_has_octal_prefix(str)) + return 8; + + /* Otherwise assume a decadic number. */ + return 10; +} + +/** Strip leading "0x" or "0X" from hex numbers or "0" from octal numbers. */ +static void str_strip_number_prefix_if_present(const char **_str, unsigned int radix) +{ + assert(radix == 16 || radix == 10 || radix == 8); + assert(_str); + + const char *str = *_str; + assert(str); + + if (radix == 16 && str_has_hex_prefix(str)) + str += 2; + else if (radix == 8 && str_has_octal_prefix(str)) + str += 1; + + /* No prefix to strip for radix == 10. */ + + *_str = str; +} + +int str_to_buf(const char *str, void *_buf, unsigned int buf_len, + unsigned int radix, unsigned int *_detected_radix) +{ + assert(radix == 0 || radix == 8 || radix == 10 || radix == 16); + + if (radix == 0) + radix = str_radix_guess(str); + + if (_detected_radix) + *_detected_radix = radix; + + str_strip_number_prefix_if_present(&str, radix); + + const size_t str_len = strlen(str); + if (str_len == 0) + return ERROR_INVALID_NUMBER; + + float factor = 0.0; if (radix == 16) factor = 0.5; /* log(16) / log(256) = 0.5 */ else if (radix == 10) @@ -243,41 +281,69 @@ int str_to_buf(const char *str, unsigned str_len, else if (radix == 8) factor = 0.375; /* log(8) / log(256) = 0.375 */ else - return 0; + assert(false); - /* copy to zero-terminated buffer */ - char *charbuf = strndup(str, str_len); + const unsigned int b256_len = ceil_f_to_u32(str_len * factor); - /* number of digits in base-256 notation */ - unsigned b256_len = ceil_f_to_u32(str_len * factor); + /* Allocate a buffer for digits in base-256 notation */ uint8_t *b256_buf = calloc(b256_len, 1); + if (!b256_buf) { + LOG_ERROR("Unable to allocate memory"); + return ERROR_FAIL; + } - /* go through zero terminated buffer - * input digits (ASCII) */ - unsigned i; - for (i = 0; charbuf[i]; i++) { - uint32_t tmp = charbuf[i]; - if ((tmp >= '0') && (tmp <= '9')) + /* Go through the zero-terminated buffer + * of input digits (ASCII) */ + for (unsigned int i = 0; str[i]; i++) { + uint32_t tmp = str[i]; + if ((tmp >= '0') && (tmp <= '9')) { tmp = (tmp - '0'); - else if ((tmp >= 'a') && (tmp <= 'f')) + } else if ((tmp >= 'a') && (tmp <= 'f')) { tmp = (tmp - 'a' + 10); - else if ((tmp >= 'A') && (tmp <= 'F')) + } else if ((tmp >= 'A') && (tmp <= 'F')) { tmp = (tmp - 'A' + 10); - else - continue; /* skip characters other than [0-9,a-f,A-F] */ + } else { + /* Characters other than [0-9,a-f,A-F] are invalid */ + free(b256_buf); + return ERROR_INVALID_NUMBER; + } - if (tmp >= radix) - continue; /* skip digits invalid for the current radix */ + if (tmp >= radix) { + /* Encountered a digit that is invalid for the current radix */ + free(b256_buf); + return ERROR_INVALID_NUMBER; + } - /* base-256 digits */ - for (unsigned j = 0; j < b256_len; j++) { + /* Add the current digit (tmp) to the intermediate result + * in b256_buf (base-256 digits) */ + for (unsigned int j = 0; j < b256_len; j++) { tmp += (uint32_t)b256_buf[j] * radix; - b256_buf[j] = (uint8_t)(tmp & 0xFF); + b256_buf[j] = (uint8_t)(tmp & 0xFFu); tmp >>= 8; } + /* The b256_t buffer is large enough to contain the whole result. */ + assert(tmp == 0); } + /* The result must not contain more bits than buf_len. */ + /* Check the whole bytes: */ + for (unsigned int j = DIV_ROUND_UP(buf_len, 8); j < b256_len; j++) { + if (b256_buf[j] != 0x0) { + free(b256_buf); + return ERROR_NUMBER_EXCEEDS_BUFFER; + } + } + /* Check the partial byte: */ + if (buf_len % 8) { + const uint8_t mask = 0xFFu << (buf_len % 8); + if ((b256_buf[(buf_len / 8)] & mask) != 0x0) { + free(b256_buf); + return ERROR_NUMBER_EXCEEDS_BUFFER; + } + } + + /* Copy the digits to the output buffer */ uint8_t *buf = _buf; for (unsigned j = 0; j < DIV_ROUND_UP(buf_len, 8); j++) { if (j < b256_len) @@ -286,14 +352,8 @@ int str_to_buf(const char *str, unsigned str_len, buf[j] = 0; } - /* mask out bits that don't belong to the buffer */ - if (buf_len % 8) - buf[(buf_len / 8)] &= 0xff >> (8 - (buf_len % 8)); - free(b256_buf); - free(charbuf); - - return i; + return ERROR_OK; } void bit_copy_queue_init(struct bit_copy_queue *q) diff --git a/src/helper/binarybuffer.h b/src/helper/binarybuffer.h index 344629681..441374330 100644 --- a/src/helper/binarybuffer.h +++ b/src/helper/binarybuffer.h @@ -14,6 +14,9 @@ #include #include +#define ERROR_INVALID_NUMBER (-1700) +#define ERROR_NUMBER_EXCEEDS_BUFFER (-1701) + /** @file * Support functions to access arbitrary bits in a byte array */ @@ -189,8 +192,18 @@ void *buf_set_ones(void *buf, unsigned size); void *buf_set_buf(const void *src, unsigned src_start, void *dst, unsigned dst_start, unsigned len); -int str_to_buf(const char *str, unsigned len, - void *bin_buf, unsigned buf_size, unsigned radix); +/** + * Parse an unsigned number (provided as a zero-terminated string) + * into a bit buffer whose size is buf_len bits. + * @param str Input number, zero-terminated string + * @param _buf Output buffer, allocated by the caller + * @param buf_len Output buffer size in bits + * @param radix Base of the input number - 16, 10, 8 or 0. + * 0 means auto-detect the radix. + */ +int str_to_buf(const char *str, void *_buf, unsigned int buf_len, + unsigned int radix, unsigned int *_detected_radix); + char *buf_to_hex_str(const void *buf, unsigned size); /* read a uint32_t from a buffer in target memory endianness */ diff --git a/src/helper/command.c b/src/helper/command.c index a775c730b..15a9b4a08 100644 --- a/src/helper/command.c +++ b/src/helper/command.c @@ -1360,6 +1360,46 @@ int command_parse_bool_arg(const char *in, bool *out) return ERROR_COMMAND_SYNTAX_ERROR; } +static const char *radix_to_str(unsigned int radix) +{ + switch (radix) { + case 16: return "hexadecimal"; + case 10: return "decadic"; + case 8: return "octal"; + } + assert(false); + return ""; +} + +COMMAND_HELPER(command_parse_str_to_buf, const char *str, void *buf, unsigned int buf_len, + unsigned int radix) +{ + assert(str); + assert(buf); + + int ret = str_to_buf(str, buf, buf_len, radix, NULL); + if (ret == ERROR_OK) + return ret; + + /* Provide a clear error message to the user */ + if (ret == ERROR_INVALID_NUMBER) { + if (radix == 0) { + /* Any radix is accepted, so don't include it in the error message. */ + command_print(CMD, "'%s' is not a valid number", str); + } else { + /* Specific radix is required - tell the user what it is. */ + command_print(CMD, "'%s' is not a valid number (requiring %s number)", + str, radix_to_str(radix)); + } + } else if (ret == ERROR_NUMBER_EXCEEDS_BUFFER) { + command_print(CMD, "Number %s exceeds %u bits", str, buf_len); + } else { + command_print(CMD, "Could not parse number '%s'", str); + } + + return ERROR_COMMAND_ARGUMENT_INVALID; +} + COMMAND_HELPER(handle_command_parse_bool, bool *out, const char *label) { switch (CMD_ARGC) { diff --git a/src/helper/command.h b/src/helper/command.h index fc26dda81..7a044e619 100644 --- a/src/helper/command.h +++ b/src/helper/command.h @@ -517,6 +517,17 @@ DECLARE_PARSE_WRAPPER(_target_addr, target_addr_t); int command_parse_bool_arg(const char *in, bool *out); COMMAND_HELPER(handle_command_parse_bool, bool *out, const char *label); +/** + * Parse a number (base 10, base 16 or base 8) and store the result + * into a bit buffer. + * + * In case of parsing error, a user-readable error message is produced. + * + * If radix = 0 is given, the function guesses the radix by looking at the number prefix. + */ +COMMAND_HELPER(command_parse_str_to_buf, const char *str, void *buf, unsigned int buf_len, + unsigned int radix); + /** parses an on/off command argument */ #define COMMAND_PARSE_ON_OFF(in, out) \ COMMAND_PARSE_BOOL(in, out, "on", "off") diff --git a/src/jtag/tcl.c b/src/jtag/tcl.c index 1a4c4b774..e53413427 100644 --- a/src/jtag/tcl.c +++ b/src/jtag/tcl.c @@ -87,8 +87,11 @@ static COMMAND_HELPER(handle_jtag_command_drscan_fields, struct scan_field *fiel LOG_ERROR("Out of memory"); return ERROR_FAIL; } + fields[field_count].out_value = t; - str_to_buf(CMD_ARGV[i + 1], strlen(CMD_ARGV[i + 1]), t, bits, 0); + int ret = CALL_COMMAND_HANDLER(command_parse_str_to_buf, CMD_ARGV[i + 1], t, bits, 0); + if (ret != ERROR_OK) + return ret; fields[field_count].in_value = t; field_count++; } diff --git a/src/target/target.c b/src/target/target.c index 1f4817d5c..8ff665f47 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -3128,11 +3128,18 @@ COMMAND_HANDLER(handle_reg_command) /* set register value */ if (CMD_ARGC == 2) { uint8_t *buf = malloc(DIV_ROUND_UP(reg->size, 8)); - if (!buf) + if (!buf) { + LOG_ERROR("Failed to allocate memory"); return ERROR_FAIL; - str_to_buf(CMD_ARGV[1], strlen(CMD_ARGV[1]), buf, reg->size, 0); + } - int retval = reg->type->set(reg, buf); + int retval = CALL_COMMAND_HANDLER(command_parse_str_to_buf, CMD_ARGV[1], buf, reg->size, 0); + if (retval != ERROR_OK) { + free(buf); + return retval; + } + + retval = reg->type->set(reg, buf); if (retval != ERROR_OK) { LOG_ERROR("Could not write to register '%s'", reg->name); } else { @@ -4788,63 +4795,64 @@ static int target_jim_get_reg(Jim_Interp *interp, int argc, return JIM_OK; } -static int target_jim_set_reg(Jim_Interp *interp, int argc, - Jim_Obj * const *argv) +COMMAND_HANDLER(handle_set_reg_command) { - if (argc != 2) { - Jim_WrongNumArgs(interp, 1, argv, "dict"); - return JIM_ERR; - } + if (CMD_ARGC != 1) + return ERROR_COMMAND_SYNTAX_ERROR; int tmp; #if JIM_VERSION >= 80 - Jim_Obj **dict = Jim_DictPairs(interp, argv[1], &tmp); + Jim_Obj **dict = Jim_DictPairs(CMD_CTX->interp, CMD_JIMTCL_ARGV[0], &tmp); if (!dict) - return JIM_ERR; + return ERROR_FAIL; #else Jim_Obj **dict; - int ret = Jim_DictPairs(interp, argv[1], &dict, &tmp); + int ret = Jim_DictPairs(CMD_CTX->interp, CMD_JIMTCL_ARGV[0], &dict, &tmp); if (ret != JIM_OK) - return ret; + return ERROR_FAIL; #endif const unsigned int length = tmp; - struct command_context *cmd_ctx = current_command_context(interp); - assert(cmd_ctx); - const struct target *target = get_current_target(cmd_ctx); + + const struct target *target = get_current_target(CMD_CTX); + assert(target); for (unsigned int i = 0; i < length; i += 2) { const char *reg_name = Jim_String(dict[i]); const char *reg_value = Jim_String(dict[i + 1]); - struct reg *reg = register_get_by_name(target->reg_cache, reg_name, - false); + struct reg *reg = register_get_by_name(target->reg_cache, reg_name, false); if (!reg || !reg->exist) { - Jim_SetResultFormatted(interp, "unknown register '%s'", reg_name); - return JIM_ERR; + command_print(CMD, "unknown register '%s'", reg_name); + return ERROR_FAIL; } uint8_t *buf = malloc(DIV_ROUND_UP(reg->size, 8)); - if (!buf) { LOG_ERROR("Failed to allocate memory"); - return JIM_ERR; + return ERROR_FAIL; } - str_to_buf(reg_value, strlen(reg_value), buf, reg->size, 0); - int retval = reg->type->set(reg, buf); + int retval = CALL_COMMAND_HANDLER(command_parse_str_to_buf, + reg_value, buf, reg->size, 0); + if (retval != ERROR_OK) { + free(buf); + return retval; + } + + retval = reg->type->set(reg, buf); free(buf); if (retval != ERROR_OK) { - Jim_SetResultFormatted(interp, "failed to set '%s' to register '%s'", + command_print(CMD, "failed to set '%s' to register '%s'", reg_value, reg_name); - return JIM_ERR; + return retval; } } - return JIM_OK; + return ERROR_OK; } /** @@ -5584,7 +5592,7 @@ static const struct command_registration target_instance_command_handlers[] = { { .name = "set_reg", .mode = COMMAND_EXEC, - .jim_handler = target_jim_set_reg, + .handler = handle_set_reg_command, .help = "Set target register values", .usage = "dict", }, @@ -6719,7 +6727,7 @@ static const struct command_registration target_exec_command_handlers[] = { { .name = "set_reg", .mode = COMMAND_EXEC, - .jim_handler = target_jim_set_reg, + .handler = handle_set_reg_command, .help = "Set target register values", .usage = "dict", },