This is an automated email from Gerrit.
"Antonio Borneo <borneo.anto...@gmail.com>" just uploaded a new patch set to
Gerrit, which you can find at https://review.openocd.org/c/openocd/+/7425
-- gerrit
commit e1e877cb797226e156d754f14fd68cccb2163b28
Author: Antonio Borneo <borneo.anto...@gmail.com>
Date: Tue Dec 27 00:56:55 2022 +0100
target: arc: rewrite command 'arc add-reg-type-struct' as COMMAND_HANDLER
Use a COMMAND_HELPER() to avoid memory leaks when the helper
COMMAND_PARSE_NUMBER() returns due to an error.
While there:
- fix potential SIGSEGV due to dereference 'type' before checking
it's not NULL;
- fix an incorrect NUL byte termination while copying to
type->data_type.id and to bitfields[cur_field].name;
- fix some coding style error;
- remove the now unused function jim_arc_read_reg_type_field().
Change-Id: I7158fd93b5d4742f11654b8ae4a7abd409ad06e2
Signed-off-by: Antonio Borneo <borneo.anto...@gmail.com>
diff --git a/src/target/arc_cmd.c b/src/target/arc_cmd.c
index 481ad9d13c..0130fb7293 100644
--- a/src/target/arc_cmd.c
+++ b/src/target/arc_cmd.c
@@ -70,51 +70,6 @@ static int jim_arc_read_reg_name_field(struct
jim_getopt_info *goi,
return e;
}
-/* Helper function to read bitfields/flags of register type. */
-static int jim_arc_read_reg_type_field(struct jim_getopt_info *goi, const char
**field_name, int *field_name_len,
- struct arc_reg_bitfield *bitfields, int cur_field, int type)
-{
- jim_wide start_pos, end_pos;
-
- int e = JIM_OK;
- if ((type == CFG_ADD_REG_TYPE_STRUCT && goi->argc < 3) ||
- (type == CFG_ADD_REG_TYPE_FLAG && goi->argc < 2)) {
- Jim_SetResultFormatted(goi->interp, "Not enough
arguments after -flag/-bitfield");
- return JIM_ERR;
- }
-
- e = jim_getopt_string(goi, field_name, field_name_len);
- if (e != JIM_OK)
- return e;
-
- /* read start position of bitfield/flag */
- e = jim_getopt_wide(goi, &start_pos);
- if (e != JIM_OK)
- return e;
-
- end_pos = start_pos;
-
- /* Check if any arguments remain,
- * set bitfields[cur_field].end if flag is multibit */
- if (goi->argc > 0)
- /* Check current argv[0], if it is equal to "-flag",
- * than bitfields[cur_field].end remains start */
- if ((strcmp(Jim_String(goi->argv[0]), "-flag") && type
== CFG_ADD_REG_TYPE_FLAG)
- || (type == CFG_ADD_REG_TYPE_STRUCT)) {
- e =
jim_getopt_wide(goi, &end_pos);
- if (e !=
JIM_OK) {
-
Jim_SetResultFormatted(goi->interp, "Error reading end position");
- return
e;
- }
- }
-
- bitfields[cur_field].bitfield.start = start_pos;
- bitfields[cur_field].bitfield.end = end_pos;
- if ((end_pos != start_pos) || (type == CFG_ADD_REG_TYPE_STRUCT))
- bitfields[cur_field].bitfield.type = REG_TYPE_INT;
- return e;
-}
-
COMMAND_HELPER(arc_handle_add_reg_type_flags_ops, struct arc_reg_data_type
*type)
{
struct reg_data_type_flags_field *fields = type->reg_type_flags_field;
@@ -257,7 +212,7 @@ enum add_reg_type_struct {
CFG_ADD_REG_TYPE_STRUCT_BITFIELD,
};
-static struct jim_nvp nvp_add_reg_type_struct_opts[] = {
+static const struct nvp nvp_add_reg_type_struct_opts[] = {
{ .name = "-name", .value = CFG_ADD_REG_TYPE_STRUCT_NAME },
{ .name = "-bitfield", .value = CFG_ADD_REG_TYPE_STRUCT_BITFIELD },
{ .name = NULL, .value = -1 }
@@ -426,53 +381,119 @@ static const struct command_registration
arc_jtag_command_group[] = {
/* This function supports only bitfields. */
-static int jim_arc_add_reg_type_struct(Jim_Interp *interp, int argc,
- Jim_Obj * const *argv)
+COMMAND_HELPER(arc_handle_add_reg_type_struct_opts, struct arc_reg_data_type
*type)
{
- struct jim_getopt_info goi;
- JIM_CHECK_RETVAL(jim_getopt_setup(&goi, interp, argc-1, argv+1));
+ struct reg_data_type_struct_field *fields = type->reg_type_struct_field;
+ struct arc_reg_bitfield *bitfields = type->bitfields;
+ struct reg_data_type_struct *struct_type = &type->data_type_struct;
+ unsigned int cur_field = 0;
- LOG_DEBUG("-");
+ while (CMD_ARGC) {
+ const struct nvp *n =
nvp_name2value(nvp_add_reg_type_struct_opts, CMD_ARGV[0]);
+ CMD_ARGC--;
+ CMD_ARGV++;
+ switch (n->value) {
+ case CFG_ADD_REG_TYPE_STRUCT_NAME:
+ if (!CMD_ARGC)
+ return ERROR_COMMAND_ARGUMENT_INVALID;
- struct command_context *ctx;
- struct target *target;
+ const char *name = CMD_ARGV[0];
+ CMD_ARGC--;
+ CMD_ARGV++;
- ctx = current_command_context(interp);
- assert(ctx);
- target = get_current_target(ctx);
- if (!target) {
- Jim_SetResultFormatted(goi.interp, "No current target");
- return JIM_ERR;
+ if (strlen(name) >= REG_TYPE_MAX_NAME_LENGTH) {
+ command_print(CMD, "Reg type name is too big.");
+ return ERROR_COMMAND_ARGUMENT_INVALID;
+ }
+
+ strcpy((void *)type->data_type.id, name);
+ break;
+
+ case CFG_ADD_REG_TYPE_STRUCT_BITFIELD:
+ if (CMD_ARGC < 3)
+ return ERROR_COMMAND_ARGUMENT_INVALID;
+
+ uint32_t start_pos, end_pos;
+ const char *field_name = CMD_ARGV[0];
+ COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], start_pos);
+ COMMAND_PARSE_NUMBER(u32, CMD_ARGV[2], end_pos);
+ CMD_ARGC -= 3;
+ CMD_ARGV += 3;
+ bitfields[cur_field].bitfield.start = start_pos;
+ bitfields[cur_field].bitfield.end = end_pos;
+ bitfields[cur_field].bitfield.type = REG_TYPE_INT;
+
+ if (strlen(field_name) >= REG_TYPE_MAX_NAME_LENGTH) {
+ command_print(CMD, "Reg type field_name is too
big.");
+ return ERROR_COMMAND_ARGUMENT_INVALID;
+ }
+
+ fields[cur_field].name = bitfields[cur_field].name;
+ strcpy(bitfields[cur_field].name, field_name);
+
+ fields[cur_field].bitfield =
&bitfields[cur_field].bitfield;
+ fields[cur_field].use_bitfields = true;
+ if (cur_field > 0)
+ fields[cur_field - 1].next = &fields[cur_field];
+ else
+ struct_type->fields = fields;
+
+ cur_field += 1;
+
+ break;
+
+ default:
+ char *list =
nvp_alloc_unknown(nvp_add_reg_type_struct_opts, NULL, n->name);
+ command_print(CMD, "%s", list);
+ free(list);
+ return ERROR_COMMAND_ARGUMENT_INVALID;
+ }
}
- int e = JIM_OK;
+ if (!type->data_type.id) {
+ command_print(CMD, "-name is a required option");
+ return ERROR_COMMAND_ARGUMENT_INVALID;
+ }
- /* Check if the amount of arguments is not zero */
- if (goi.argc <= 0) {
- Jim_SetResultFormatted(goi.interp, "The command has no
arguments");
- return JIM_ERR;
+ return ERROR_OK;
+}
+
+COMMAND_HANDLER(arc_handle_add_reg_type_struct)
+{
+ int retval;
+
+ LOG_DEBUG("-");
+
+ struct target *target = get_current_target(CMD_CTX);
+ if (!target) {
+ command_print(CMD, "No current target");
+ return ERROR_FAIL;
}
+ /* Check if the amount of arguments is not zero */
+ if (CMD_ARGC == 0)
+ return ERROR_COMMAND_SYNTAX_ERROR;
+
/* Estimate number of registers as (argc - 2)/4 as each -bitfield
option has 3
* arguments while -name is required. */
- unsigned int fields_sz = (goi.argc - 2) / 4;
- unsigned int cur_field = 0;
+ unsigned int fields_sz = (CMD_ARGC - 2) / 4;
/* The maximum amount of bitfields is 32 */
if (fields_sz > 32) {
- Jim_SetResultFormatted(goi.interp, "The amount of
bitfields exceed 32");
- return JIM_ERR;
+ command_print(CMD, "The amount of bitfields exceed 32");
+ return ERROR_COMMAND_ARGUMENT_INVALID;
}
struct arc_reg_data_type *type = calloc(1, sizeof(*type));
- struct reg_data_type_struct *struct_type = &type->data_type_struct;
struct reg_data_type_struct_field *fields = calloc(fields_sz,
sizeof(*fields));
- type->reg_type_struct_field = fields;
struct arc_reg_bitfield *bitfields = calloc(fields_sz,
sizeof(*bitfields));
- if (!(type && fields && bitfields)) {
- Jim_SetResultFormatted(goi.interp, "Failed to allocate
memory.");
+ if (!type || !fields || !bitfields) {
+ LOG_ERROR("Out of memory");
+ retval = ERROR_FAIL;
goto fail;
}
+ struct reg_data_type_struct *struct_type = &type->data_type_struct;
+ type->reg_type_struct_field = fields;
/* Initialize type */
type->data_type.id = type->data_type_id;
@@ -482,91 +503,22 @@ static int jim_arc_add_reg_type_struct(Jim_Interp
*interp, int argc,
type->data_type.reg_type_struct = struct_type;
struct_type->size = 4; /* For now ARC has only 32-bit registers */
- while (goi.argc > 0 && e == JIM_OK) {
- struct jim_nvp *n;
- e = jim_getopt_nvp(&goi, nvp_add_reg_type_struct_opts, &n);
- if (e != JIM_OK) {
- jim_getopt_nvp_unknown(&goi,
nvp_add_reg_type_struct_opts, 0);
- continue;
- }
-
- switch (n->value) {
- case CFG_ADD_REG_TYPE_STRUCT_NAME:
- {
- const char *name = NULL;
- int name_len = 0;
-
- e = jim_arc_read_reg_name_field(&goi, &name,
&name_len);
- if (e != JIM_OK) {
- Jim_SetResultFormatted(goi.interp,
"Unable to read reg name.");
- goto fail;
- }
-
- if (name_len > REG_TYPE_MAX_NAME_LENGTH) {
- Jim_SetResultFormatted(goi.interp, "Reg
type name is too big.");
- goto fail;
- }
-
- strncpy((void *)type->data_type.id, name,
name_len);
- if (!type->data_type.id) {
- Jim_SetResultFormatted(goi.interp,
"Unable to setup reg type name.");
- goto fail;
- }
-
- break;
- }
- case CFG_ADD_REG_TYPE_STRUCT_BITFIELD:
- {
- const char *field_name = NULL;
- int field_name_len = 0;
- e = jim_arc_read_reg_type_field(&goi,
&field_name, &field_name_len, bitfields,
-
cur_field, CFG_ADD_REG_TYPE_STRUCT);
- if (e != JIM_OK) {
- Jim_SetResultFormatted(goi.interp,
"Unable to add reg_type_struct field.");
- goto fail;
- }
-
- if (field_name_len > REG_TYPE_MAX_NAME_LENGTH) {
- Jim_SetResultFormatted(goi.interp, "Reg
type field_name_len is too big.");
- goto fail;
- }
-
- fields[cur_field].name =
bitfields[cur_field].name;
- strncpy(bitfields[cur_field].name, field_name,
field_name_len);
- if (!fields[cur_field].name) {
- Jim_SetResultFormatted(goi.interp,
"Unable to setup field name. ");
- goto fail;
- }
-
- fields[cur_field].bitfield =
&(bitfields[cur_field].bitfield);
- fields[cur_field].use_bitfields = true;
- if (cur_field > 0)
- fields[cur_field - 1].next =
&(fields[cur_field]);
- else
- struct_type->fields = fields;
-
- cur_field += 1;
-
- break;
- }
- }
- }
-
- if (!type->data_type.id) {
- Jim_SetResultFormatted(goi.interp, "-name is a required
option");
+ retval = CALL_COMMAND_HANDLER(arc_handle_add_reg_type_struct_opts,
type);
+ if (retval != ERROR_OK)
goto fail;
- }
arc_reg_data_type_add(target, type);
+
LOG_DEBUG("added struct type {name=%s}", type->data_type.id);
- return JIM_OK;
+
+ return ERROR_OK;
fail:
- free(type);
- free(fields);
- free(bitfields);
+ free(type);
+ free(fields);
+ free(bitfields);
- return JIM_ERR;
+ return retval;
}
/* Add register */
@@ -927,7 +879,7 @@ static const struct command_registration
arc_core_command_handlers[] = {
},
{
.name = "add-reg-type-struct",
- .jim_handler = jim_arc_add_reg_type_struct,
+ .handler = arc_handle_add_reg_type_struct,
.mode = COMMAND_CONFIG,
.usage = "-name <string> -bitfield <name> <start> <end> "
"[-bitfield <name> <start> <end>]...",
--
