target: arc: rewrite command 'arc add-reg' as COMMAND_HANDLER 26/7426/4
authorAntonio Borneo <borneo.antonio@gmail.com>
Tue, 27 Dec 2022 01:38:25 +0000 (02:38 +0100)
committerAntonio Borneo <borneo.antonio@gmail.com>
Fri, 3 Feb 2023 22:48:48 +0000 (22:48 +0000)
While there, fix some coding style error and remove the now unused
function jim_arc_read_reg_name_field() and the macro
JIM_CHECK_RETVAL().

Change-Id: I140b4b929978b2936f2310e0b7d1735ba726c517
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/7426
Tested-by: jenkins
Reviewed-by: Evgeniy Didin <didin@synopsys.com>
src/target/arc.h
src/target/arc_cmd.c

index bb70a598e100a7b4feea7c35be32145485bf66ac..a351802ac6ffe251dfd73147d256bb91b3bdbfd4 100644 (file)
@@ -253,16 +253,6 @@ struct arc_common {
                }                               \
        } while (0)
 
-#define JIM_CHECK_RETVAL(action)               \
-       do {                                    \
-               int __retval = (action);        \
-               if (__retval != JIM_OK) {       \
-                       LOG_DEBUG("error while calling \"%s\"", \
-                               # action);     \
-                       return __retval;        \
-               }                               \
-       } while (0)
-
 static inline struct arc_common *target_to_arc(struct target *target)
 {
        return target->arch_info;
index 3b0bf76f8000351f004773093a775ace75ac0863..e7760b0378dd6c695967c0739de1fd1b785d9c71 100644 (file)
@@ -55,21 +55,6 @@ static const char *validate_register(const struct arc_reg_desc * const reg, bool
        return NULL;
 }
 
-/* Helper function to read the name of register type or register from
- * configure files  */
-static int jim_arc_read_reg_name_field(struct jim_getopt_info *goi,
-       const char **name, int *name_len)
-{
-       int e = JIM_OK;
-
-       if (!goi->argc) {
-               Jim_WrongNumArgs(goi->interp, goi->argc, goi->argv, "-name <name> ...");
-               return JIM_ERR;
-       }
-       e = jim_getopt_string(goi, name, name_len);
-       return e;
-}
-
 static 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;
@@ -528,7 +513,7 @@ enum opts_add_reg {
        CFG_ADD_REG_GENERAL,
 };
 
-static struct jim_nvp opts_nvp_add_reg[] = {
+static const struct nvp opts_nvp_add_reg[] = {
        { .name = "-name",    .value = CFG_ADD_REG_NAME },
        { .name = "-num",     .value = CFG_ADD_REG_ARCH_NUM },
        { .name = "-core",    .value = CFG_ADD_REG_IS_CORE },
@@ -546,155 +531,133 @@ void free_reg_desc(struct arc_reg_desc *r)
        free(r);
 }
 
-static int jim_arc_add_reg(Jim_Interp *interp, int argc, Jim_Obj * const *argv)
+static COMMAND_HELPER(arc_handle_add_reg_do, struct arc_reg_desc *reg)
 {
-       struct jim_getopt_info goi;
-       JIM_CHECK_RETVAL(jim_getopt_setup(&goi, interp, argc-1, argv+1));
-
-       struct arc_reg_desc *reg = calloc(1, sizeof(*reg));
-       if (!reg) {
-               Jim_SetResultFormatted(goi.interp, "Failed to allocate memory.");
-               return JIM_ERR;
-       }
-
        /* There is no architecture number that we could treat as invalid, so
         * separate variable required to ensure that arch num has been set. */
        bool arch_num_set = false;
        const char *type_name = "int"; /* Default type */
-       int type_name_len = strlen(type_name);
-       int e = ERROR_OK;
 
        /* At least we need to specify 4 parameters: name, number and gdb_feature,
         * which means there should be 6 arguments. Also there can be additional parameters
         * "-type <type>", "-g" and  "-core" or "-bcr" which makes maximum 10 parameters. */
-       if (goi.argc < 6 || goi.argc > 10) {
-               free_reg_desc(reg);
-               Jim_SetResultFormatted(goi.interp,
-                       "Should be at least 6 arguments and not greater than 10: "
-                       " -name <name> -num <num> -feature <gdb_feature> "
-                       " [-type <type_name>] [-core|-bcr] [-g].");
-               return JIM_ERR;
-       }
+       if (CMD_ARGC < 6 || CMD_ARGC > 10)
+               return ERROR_COMMAND_SYNTAX_ERROR;
 
        /* Parse options. */
-       while (goi.argc > 0) {
-               struct jim_nvp *n;
-               e = jim_getopt_nvp(&goi, opts_nvp_add_reg, &n);
-               if (e != JIM_OK) {
-                       jim_getopt_nvp_unknown(&goi, opts_nvp_add_reg, 0);
-                       free_reg_desc(reg);
-                       return e;
-               }
-
+       while (CMD_ARGC) {
+               const struct nvp *n = nvp_name2value(opts_nvp_add_reg, CMD_ARGV[0]);
+               CMD_ARGC--;
+               CMD_ARGV++;
                switch (n->value) {
-                       case CFG_ADD_REG_NAME:
-                       {
-                               const char *reg_name = NULL;
-                               int reg_name_len = 0;
-
-                               e = jim_arc_read_reg_name_field(&goi, &reg_name, &reg_name_len);
-                               if (e != JIM_OK) {
-                                       Jim_SetResultFormatted(goi.interp, "Unable to read register name.");
-                                       free_reg_desc(reg);
-                                       return e;
-                               }
-
-                               reg->name = strndup(reg_name, reg_name_len);
-                               break;
-                       }
-                       case CFG_ADD_REG_IS_CORE:
-                               reg->is_core = true;
-                               break;
-                       case CFG_ADD_REG_IS_BCR:
-                               reg->is_bcr = true;
-                               break;
-                       case CFG_ADD_REG_ARCH_NUM:
-                       {
-                               jim_wide archnum;
-
-                               if (!goi.argc) {
-                                       free_reg_desc(reg);
-                                       Jim_WrongNumArgs(interp, goi.argc, goi.argv, "-num <int> ...");
-                                       return JIM_ERR;
-                               }
-
-                               e = jim_getopt_wide(&goi, &archnum);
-                               if (e != JIM_OK) {
-                                       free_reg_desc(reg);
-                                       return e;
-                               }
-
-                               reg->arch_num = archnum;
-                               arch_num_set = true;
-                               break;
+               case CFG_ADD_REG_NAME:
+                       if (!CMD_ARGC)
+                               return ERROR_COMMAND_ARGUMENT_INVALID;
+
+                       reg->name = strdup(CMD_ARGV[0]);
+                       if (!reg->name) {
+                               LOG_ERROR("Out of memory");
+                               return ERROR_FAIL;
                        }
-                       case CFG_ADD_REG_GDB_FEATURE:
-                       {
-                               const char *feature = NULL;
-                               int feature_len = 0;
-
-                               e = jim_arc_read_reg_name_field(&goi, &feature, &feature_len);
-                               if (e != JIM_OK) {
-                                       Jim_SetResultFormatted(goi.interp, "Unable to read gdb_feature.");
-                                       free_reg_desc(reg);
-                                       return e;
-                               }
-
-                               reg->gdb_xml_feature = strndup(feature, feature_len);
-                               break;
+
+                       CMD_ARGC--;
+                       CMD_ARGV++;
+                       break;
+
+               case CFG_ADD_REG_IS_CORE:
+                       reg->is_core = true;
+                       break;
+
+               case CFG_ADD_REG_IS_BCR:
+                       reg->is_bcr = true;
+                       break;
+
+               case CFG_ADD_REG_ARCH_NUM:
+                       if (!CMD_ARGC)
+                               return ERROR_COMMAND_ARGUMENT_INVALID;
+
+                       COMMAND_PARSE_NUMBER(u32, CMD_ARGV[0], reg->arch_num);
+                       CMD_ARGC--;
+                       CMD_ARGV++;
+
+                       arch_num_set = true;
+                       break;
+
+               case CFG_ADD_REG_GDB_FEATURE:
+                       if (!CMD_ARGC)
+                               return ERROR_COMMAND_ARGUMENT_INVALID;
+
+                       reg->gdb_xml_feature = strdup(CMD_ARGV[0]);
+                       if (!reg->gdb_xml_feature) {
+                               LOG_ERROR("Out of memory");
+                               return ERROR_FAIL;
                        }
-                       case CFG_ADD_REG_TYPE:
-                               e = jim_arc_read_reg_name_field(&goi, &type_name, &type_name_len);
-                               if (e != JIM_OK) {
-                                       Jim_SetResultFormatted(goi.interp, "Unable to read register type.");
-                                       free_reg_desc(reg);
-                                       return e;
-                               }
-
-                               break;
-                       case CFG_ADD_REG_GENERAL:
-                               reg->is_general = true;
-                               break;
-                       default:
-                               LOG_DEBUG("Error: Unknown parameter");
-                               free_reg_desc(reg);
-                               return JIM_ERR;
+
+                       CMD_ARGC--;
+                       CMD_ARGV++;
+                       break;
+
+               case CFG_ADD_REG_TYPE:
+                       if (!CMD_ARGC)
+                               return ERROR_COMMAND_ARGUMENT_INVALID;
+
+                       type_name = CMD_ARGV[0];
+                       CMD_ARGC--;
+                       CMD_ARGV++;
+                       break;
+
+               case CFG_ADD_REG_GENERAL:
+                       reg->is_general = true;
+                       break;
+
+               default:
+                       nvp_unknown_command_print(CMD, opts_nvp_add_reg, NULL, CMD_ARGV[-1]);
+                       return ERROR_COMMAND_ARGUMENT_INVALID;
                }
        }
 
        /* Check that required fields are set */
        const char * const errmsg = validate_register(reg, arch_num_set);
        if (errmsg) {
-               Jim_SetResultFormatted(goi.interp, errmsg);
-               free_reg_desc(reg);
-               return JIM_ERR;
+               command_print(CMD, "%s", errmsg);
+               return ERROR_COMMAND_ARGUMENT_INVALID;
        }
 
        /* Add new register */
-       struct command_context *ctx;
-       struct target *target;
-
-       ctx = current_command_context(interp);
-       assert(ctx);
-       target = get_current_target(ctx);
+       struct target *target = get_current_target(CMD_CTX);
        if (!target) {
-               Jim_SetResultFormatted(goi.interp, "No current target");
-               free_reg_desc(reg);
-               return JIM_ERR;
+               command_print(CMD, "No current target");
+               return ERROR_FAIL;
        }
 
        reg->target = target;
 
-       e = arc_reg_add(target, reg, type_name, type_name_len);
-       if (e == ERROR_ARC_REGTYPE_NOT_FOUND) {
-               Jim_SetResultFormatted(goi.interp,
+       int retval = arc_reg_add(target, reg, type_name, strlen(type_name));
+       if (retval == ERROR_ARC_REGTYPE_NOT_FOUND) {
+               command_print(CMD,
                        "Cannot find type `%s' for register `%s'.",
                        type_name, reg->name);
+               return retval;
+       }
+
+       return ERROR_OK;
+}
+
+COMMAND_HANDLER(arc_handle_add_reg)
+{
+       struct arc_reg_desc *reg = calloc(1, sizeof(*reg));
+       if (!reg) {
+               LOG_ERROR("Out of memory");
+               return ERROR_FAIL;
+       }
+
+       int retval = CALL_COMMAND_HANDLER(arc_handle_add_reg_do, reg);
+       if (retval != ERROR_OK) {
                free_reg_desc(reg);
-               return JIM_ERR;
+               return retval;
        }
 
-       return e;
+       return ERROR_OK;
 }
 
 /* arc set-reg-exists ($reg_name)+
@@ -887,7 +850,7 @@ static const struct command_registration arc_core_command_handlers[] = {
        },
        {
                .name = "add-reg",
-               .jim_handler = jim_arc_add_reg,
+               .handler = arc_handle_add_reg,
                .mode = COMMAND_CONFIG,
                .usage = "-name <string> -num <int> -feature <string> [-gdbnum <int>] "
                        "[-core|-bcr] [-type <type_name>] [-g]",

Linking to existing account procedure

If you already have an account and want to add another login method you MUST first sign in with your existing account and then change URL to read https://review.openocd.org/login/?link to get to this page again but this time it'll work for linking. Thank you.

SSH host keys fingerprints

1024 SHA256:YKx8b7u5ZWdcbp7/4AeXNaqElP49m6QrwfXaqQGJAOk gerrit-code-review@openocd.zylin.com (DSA)
384 SHA256:jHIbSQa4REvwCFG4cq5LBlBLxmxSqelQPem/EXIrxjk gerrit-code-review@openocd.org (ECDSA)
521 SHA256:UAOPYkU9Fjtcao0Ul/Rrlnj/OsQvt+pgdYSZ4jOYdgs gerrit-code-review@openocd.org (ECDSA)
256 SHA256:A13M5QlnozFOvTllybRZH6vm7iSt0XLxbA48yfc2yfY gerrit-code-review@openocd.org (ECDSA)
256 SHA256:spYMBqEYoAOtK7yZBrcwE8ZpYt6b68Cfh9yEVetvbXg gerrit-code-review@openocd.org (ED25519)
+--[ED25519 256]--+
|=..              |
|+o..   .         |
|*.o   . .        |
|+B . . .         |
|Bo. = o S        |
|Oo.+ + =         |
|oB=.* = . o      |
| =+=.+   + E     |
|. .=o   . o      |
+----[SHA256]-----+
2048 SHA256:0Onrb7/PHjpo6iVZ7xQX2riKN83FJ3KGU0TvI0TaFG4 gerrit-code-review@openocd.zylin.com (RSA)