jtag: rewrite commands 'jtag newtap' and 'swd newdap' as COMMAND_HANDLER 31/7431/4
authorAntonio Borneo <borneo.antonio@gmail.com>
Mon, 2 Jan 2023 00:11:44 +0000 (01:11 +0100)
committerAntonio Borneo <borneo.antonio@gmail.com>
Sat, 27 May 2023 06:42:01 +0000 (06:42 +0000)
While there:
- fix memory leak in case of error on values tap->chip,
  tap->tapname, tap->expected_ids;
- check for out of memory error;
- fix minor coding style issue;
- add the missing .usage field;
- remove functions not in use anymore.

Change-Id: I1c8c3ffeb324e9eacb919c7e0d94fd72122c9a81
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/7431
Tested-by: jenkins
src/jtag/hla/hla_transport.c
src/jtag/jtag.h
src/jtag/tcl.c
src/target/adi_v5_dapdirect.c
src/target/adi_v5_swd.c

index 004e9f0c53da63cbc8cf400fa40e7c040b7a3a72..72d1edc6d6f4b9df8bfd4936cd4a8a3140e812ef 100644 (file)
@@ -37,8 +37,15 @@ static const struct command_registration hl_swd_transport_subcommand_handlers[]
        {
         .name = "newdap",
         .mode = COMMAND_CONFIG,
-        .jim_handler = jim_jtag_newtap,
+        .handler = handle_jtag_newtap,
         .help = "declare a new SWD DAP",
+        .usage = "basename dap_type ['-irlen' count] "
+                       "['-enable'|'-disable'] "
+                       "['-expected_id' number] "
+                       "['-ignore-version'] "
+                       "['-ignore-bypass'] "
+                       "['-ircapture' number] "
+                       "['-mask' number]",
         },
        COMMAND_REGISTRATION_DONE
 };
@@ -58,11 +65,16 @@ static const struct command_registration hl_transport_jtag_subcommand_handlers[]
        {
         .name = "newtap",
         .mode = COMMAND_CONFIG,
-        .jim_handler = jim_jtag_newtap,
+        .handler = handle_jtag_newtap,
         .help = "Create a new TAP instance named basename.tap_type, "
         "and appends it to the scan chain.",
         .usage = "basename tap_type '-irlen' count "
-        "['-expected_id' number]",
+                       "['-enable'|'-disable'] "
+                       "['-expected_id' number] "
+                       "['-ignore-version'] "
+                       "['-ignore-bypass'] "
+                       "['-ircapture' number] "
+                       "['-mask' number]",
         },
        {
         .name = "init",
index 4f94e991357a57a90727ffd217696671156ccf1d..04d1b4a2b53c331e116502ea37a6b8a0a7f55d7a 100644 (file)
@@ -12,6 +12,7 @@
 #define OPENOCD_JTAG_JTAG_H
 
 #include <helper/binarybuffer.h>
+#include <helper/command.h>
 #include <helper/log.h>
 #include <helper/replacements.h>
 
@@ -602,6 +603,6 @@ void jtag_poll_unmask(bool saved);
 
 #include <jtag/minidriver.h>
 
-int jim_jtag_newtap(Jim_Interp *interp, int argc, Jim_Obj *const *argv);
+__COMMAND_HANDLER(handle_jtag_newtap);
 
 #endif /* OPENOCD_JTAG_JTAG_H */
index 934b603524170eb225bd9bc28fcf19ccaf732a8a..700772ea4b17cdae96ccf64bb357b8c712c9597e 100644 (file)
@@ -31,6 +31,8 @@
 #include <strings.h>
 #endif
 
+#include <helper/command.h>
+#include <helper/nvp.h>
 #include <helper/time_support.h>
 #include "transport/transport.h"
 
@@ -376,39 +378,6 @@ static int jtag_tap_configure_cmd(struct jim_getopt_info *goi, struct jtag_tap *
        return JIM_OK;
 }
 
-static int is_bad_irval(int ir_length, jim_wide w)
-{
-       jim_wide v = 1;
-
-       v <<= ir_length;
-       v -= 1;
-       v = ~v;
-       return (w & v) != 0;
-}
-
-static int jim_newtap_expected_id(struct jim_nvp *n, struct jim_getopt_info *goi,
-       struct jtag_tap *tap)
-{
-       jim_wide w;
-       int e = jim_getopt_wide(goi, &w);
-       if (e != JIM_OK) {
-               Jim_SetResultFormatted(goi->interp, "option: %s bad parameter", n->name);
-               return e;
-       }
-
-       uint32_t *p = realloc(tap->expected_ids,
-                             (tap->expected_ids_cnt + 1) * sizeof(uint32_t));
-       if (!p) {
-               Jim_SetResultFormatted(goi->interp, "no memory");
-               return JIM_ERR;
-       }
-
-       tap->expected_ids = p;
-       tap->expected_ids[tap->expected_ids_cnt++] = w;
-
-       return JIM_OK;
-}
-
 #define NTAP_OPT_IRLEN     0
 #define NTAP_OPT_IRMASK    1
 #define NTAP_OPT_IRCAPTURE 2
@@ -418,166 +387,155 @@ static int jim_newtap_expected_id(struct jim_nvp *n, struct jim_getopt_info *goi
 #define NTAP_OPT_VERSION   6
 #define NTAP_OPT_BYPASS    7
 
-static int jim_newtap_ir_param(struct jim_nvp *n, struct jim_getopt_info *goi,
-       struct jtag_tap *tap)
-{
-       jim_wide w;
-       int e = jim_getopt_wide(goi, &w);
-       if (e != JIM_OK) {
-               Jim_SetResultFormatted(goi->interp,
-                       "option: %s bad parameter", n->name);
-               return e;
-       }
-       switch (n->value) {
-           case NTAP_OPT_IRLEN:
-                   if (w > (jim_wide) (8 * sizeof(tap->ir_capture_value))) {
-                           LOG_WARNING("%s: huge IR length %d",
-                                   tap->dotted_name, (int) w);
-                   }
-                   tap->ir_length = w;
-                   break;
-           case NTAP_OPT_IRMASK:
-                   if (is_bad_irval(tap->ir_length, w)) {
-                           LOG_ERROR("%s: IR mask %x too big",
-                                   tap->dotted_name,
-                                   (int) w);
-                           return JIM_ERR;
-                   }
-                   if ((w & 3) != 3)
-                           LOG_WARNING("%s: nonstandard IR mask", tap->dotted_name);
-                   tap->ir_capture_mask = w;
-                   break;
-           case NTAP_OPT_IRCAPTURE:
-                   if (is_bad_irval(tap->ir_length, w)) {
-                           LOG_ERROR("%s: IR capture %x too big",
-                                   tap->dotted_name, (int) w);
-                           return JIM_ERR;
-                   }
-                   if ((w & 3) != 1)
-                           LOG_WARNING("%s: nonstandard IR value",
-                                   tap->dotted_name);
-                   tap->ir_capture_value = w;
-                   break;
-           default:
-                   return JIM_ERR;
-       }
-       return JIM_OK;
-}
+static const struct nvp jtag_newtap_opts[] = {
+       { .name = "-irlen",          .value = NTAP_OPT_IRLEN },
+       { .name = "-irmask",         .value = NTAP_OPT_IRMASK },
+       { .name = "-ircapture",      .value = NTAP_OPT_IRCAPTURE },
+       { .name = "-enable",         .value = NTAP_OPT_ENABLED },
+       { .name = "-disable",        .value = NTAP_OPT_DISABLED },
+       { .name = "-expected-id",    .value = NTAP_OPT_EXPECTED_ID },
+       { .name = "-ignore-version", .value = NTAP_OPT_VERSION },
+       { .name = "-ignore-bypass",  .value = NTAP_OPT_BYPASS },
+       { .name = NULL,              .value = -1 },
+};
 
-static int jim_newtap_cmd(struct jim_getopt_info *goi)
+static COMMAND_HELPER(handle_jtag_newtap_args, struct jtag_tap *tap)
 {
-       struct jtag_tap *tap;
-       int x;
-       int e;
-       struct jim_nvp *n;
-       char *cp;
-       const struct jim_nvp opts[] = {
-               { .name = "-irlen",       .value = NTAP_OPT_IRLEN },
-               { .name = "-irmask",       .value = NTAP_OPT_IRMASK },
-               { .name = "-ircapture",       .value = NTAP_OPT_IRCAPTURE },
-               { .name = "-enable",       .value = NTAP_OPT_ENABLED },
-               { .name = "-disable",       .value = NTAP_OPT_DISABLED },
-               { .name = "-expected-id",       .value = NTAP_OPT_EXPECTED_ID },
-               { .name = "-ignore-version",       .value = NTAP_OPT_VERSION },
-               { .name = "-ignore-bypass",       .value = NTAP_OPT_BYPASS },
-               { .name = NULL,       .value = -1 },
-       };
-
-       tap = calloc(1, sizeof(struct jtag_tap));
-       if (!tap) {
-               Jim_SetResultFormatted(goi->interp, "no memory");
-               return JIM_ERR;
-       }
+       /* we expect CHIP + TAP + OPTIONS */
+       if (CMD_ARGC < 2)
+               return ERROR_COMMAND_SYNTAX_ERROR;
 
-       /*
-        * we expect CHIP + TAP + OPTIONS
-        * */
-       if (goi->argc < 3) {
-               Jim_SetResultFormatted(goi->interp, "Missing CHIP TAP OPTIONS ....");
-               free(tap);
-               return JIM_ERR;
+       tap->chip = strdup(CMD_ARGV[0]);
+       tap->tapname = strdup(CMD_ARGV[1]);
+       tap->dotted_name = alloc_printf("%s.%s", CMD_ARGV[0], CMD_ARGV[1]);
+       if (!tap->chip || !tap->tapname || !tap->dotted_name) {
+               LOG_ERROR("Out of memory");
+               return ERROR_FAIL;
        }
-
-       const char *tmp;
-       jim_getopt_string(goi, &tmp, NULL);
-       tap->chip = strdup(tmp);
-
-       jim_getopt_string(goi, &tmp, NULL);
-       tap->tapname = strdup(tmp);
-
-       /* name + dot + name + null */
-       x = strlen(tap->chip) + 1 + strlen(tap->tapname) + 1;
-       cp = malloc(x);
-       sprintf(cp, "%s.%s", tap->chip, tap->tapname);
-       tap->dotted_name = cp;
+       CMD_ARGC -= 2;
+       CMD_ARGV += 2;
 
        LOG_DEBUG("Creating New Tap, Chip: %s, Tap: %s, Dotted: %s, %d params",
-               tap->chip, tap->tapname, tap->dotted_name, goi->argc);
+                 tap->chip, tap->tapname, tap->dotted_name, CMD_ARGC);
 
-       /* IEEE specifies that the two LSBs of an IR scan are 01, so make
+       /*
+        * IEEE specifies that the two LSBs of an IR scan are 01, so make
         * that the default.  The "-ircapture" and "-irmask" options are only
         * needed to cope with nonstandard TAPs, or to specify more bits.
         */
        tap->ir_capture_mask = 0x03;
        tap->ir_capture_value = 0x01;
 
-       while (goi->argc) {
-               e = jim_getopt_nvp(goi, opts, &n);
-               if (e != JIM_OK) {
-                       jim_getopt_nvp_unknown(goi, opts, 0);
-                       free(cp);
-                       free(tap);
-                       return e;
-               }
-               LOG_DEBUG("Processing option: %s", n->name);
+       while (CMD_ARGC) {
+               const struct nvp *n = nvp_name2value(jtag_newtap_opts, CMD_ARGV[0]);
+               CMD_ARGC--;
+               CMD_ARGV++;
                switch (n->value) {
-                   case NTAP_OPT_ENABLED:
-                           tap->disabled_after_reset = false;
-                           break;
-                   case NTAP_OPT_DISABLED:
-                           tap->disabled_after_reset = true;
-                           break;
-                   case NTAP_OPT_EXPECTED_ID:
-                           e = jim_newtap_expected_id(n, goi, tap);
-                           if (e != JIM_OK) {
-                                   free(cp);
-                                   free(tap);
-                                   return e;
-                           }
-                           break;
-                   case NTAP_OPT_IRLEN:
-                   case NTAP_OPT_IRMASK:
-                   case NTAP_OPT_IRCAPTURE:
-                           e = jim_newtap_ir_param(n, goi, tap);
-                           if (e != JIM_OK) {
-                                   free(cp);
-                                   free(tap);
-                                   return e;
-                           }
-                           break;
-                   case NTAP_OPT_VERSION:
-                           tap->ignore_version = true;
-                           break;
-                   case NTAP_OPT_BYPASS:
-                           tap->ignore_bypass = true;
-                           break;
-               }       /* switch (n->value) */
-       }       /* while (goi->argc) */
+           case NTAP_OPT_ENABLED:
+                   tap->disabled_after_reset = false;
+                   break;
+
+           case NTAP_OPT_DISABLED:
+                   tap->disabled_after_reset = true;
+                   break;
+
+               case NTAP_OPT_EXPECTED_ID:
+                       if (!CMD_ARGC)
+                               return ERROR_COMMAND_ARGUMENT_INVALID;
+
+                       tap->expected_ids = realloc(tap->expected_ids,
+                                                                               (tap->expected_ids_cnt + 1) * sizeof(uint32_t));
+                       if (!tap->expected_ids) {
+                               LOG_ERROR("Out of memory");
+                               return ERROR_FAIL;
+                       }
+
+                       uint32_t id;
+                       COMMAND_PARSE_NUMBER(u32, CMD_ARGV[0], id);
+                       CMD_ARGC--;
+                       CMD_ARGV++;
+                       tap->expected_ids[tap->expected_ids_cnt++] = id;
+
+                       break;
+
+               case NTAP_OPT_IRLEN:
+                       if (!CMD_ARGC)
+                               return ERROR_COMMAND_ARGUMENT_INVALID;
+
+                       COMMAND_PARSE_NUMBER(int, CMD_ARGV[0], tap->ir_length);
+                       CMD_ARGC--;
+                       CMD_ARGV++;
+                       if (tap->ir_length > (int)(8 * sizeof(tap->ir_capture_value)))
+                               LOG_WARNING("%s: huge IR length %d", tap->dotted_name, tap->ir_length);
+                       break;
+
+               case NTAP_OPT_IRMASK:
+                       if (!CMD_ARGC)
+                               return ERROR_COMMAND_ARGUMENT_INVALID;
+
+                       COMMAND_PARSE_NUMBER(u32, CMD_ARGV[0], tap->ir_capture_mask);
+                       CMD_ARGC--;
+                       CMD_ARGV++;
+                       if ((tap->ir_capture_mask & 3) != 3)
+                               LOG_WARNING("%s: nonstandard IR mask", tap->dotted_name);
+                       break;
+
+               case NTAP_OPT_IRCAPTURE:
+                       if (!CMD_ARGC)
+                               return ERROR_COMMAND_ARGUMENT_INVALID;
+
+                       COMMAND_PARSE_NUMBER(u32, CMD_ARGV[0], tap->ir_capture_value);
+                       CMD_ARGC--;
+                       CMD_ARGV++;
+                       if ((tap->ir_capture_value & 3) != 1)
+                               LOG_WARNING("%s: nonstandard IR value", tap->dotted_name);
+                       break;
+
+               case NTAP_OPT_VERSION:
+                       tap->ignore_version = true;
+                       break;
+
+               case NTAP_OPT_BYPASS:
+                       tap->ignore_bypass = true;
+                       break;
+
+               default:
+                       nvp_unknown_command_print(CMD, jtag_newtap_opts, NULL, CMD_ARGV[-1]);
+                       return ERROR_COMMAND_ARGUMENT_INVALID;
+               }
+       }
 
        /* default is enabled-after-reset */
        tap->enabled = !tap->disabled_after_reset;
 
-       /* Did all the required option bits get cleared? */
-       if (!transport_is_jtag() || tap->ir_length != 0) {
-               jtag_tap_init(tap);
-               return JIM_OK;
+       if (transport_is_jtag() && tap->ir_length == 0) {
+               command_print(CMD, "newtap: %s missing IR length", tap->dotted_name);
+               return ERROR_COMMAND_ARGUMENT_INVALID;
+       }
+
+       return ERROR_OK;
+}
+
+__COMMAND_HANDLER(handle_jtag_newtap)
+{
+       struct jtag_tap *tap = calloc(1, sizeof(struct jtag_tap));
+       if (!tap) {
+               LOG_ERROR("Out of memory");
+               return ERROR_FAIL;
+       }
+
+       int retval = CALL_COMMAND_HANDLER(handle_jtag_newtap_args, tap);
+       if (retval != ERROR_OK) {
+               free(tap->chip);
+               free(tap->tapname);
+               free(tap->dotted_name);
+               free(tap->expected_ids);
+               free(tap);
+               return retval;
        }
 
-       Jim_SetResultFormatted(goi->interp,
-               "newtap: %s missing IR length",
-               tap->dotted_name);
-       jtag_tap_free(tap);
-       return JIM_ERR;
+       jtag_tap_init(tap);
+       return ERROR_OK;
 }
 
 static void jtag_tap_handle_event(struct jtag_tap *tap, enum jtag_event e)
@@ -643,13 +601,6 @@ COMMAND_HANDLER(handle_jtag_arp_init_reset)
        return ERROR_OK;
 }
 
-int jim_jtag_newtap(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
-{
-       struct jim_getopt_info goi;
-       jim_getopt_setup(&goi, interp, argc-1, argv + 1);
-       return jim_newtap_cmd(&goi);
-}
-
 static bool jtag_tap_enable(struct jtag_tap *t)
 {
        if (t->enabled)
@@ -798,7 +749,7 @@ static const struct command_registration jtag_subcommand_handlers[] = {
        {
                .name = "newtap",
                .mode = COMMAND_CONFIG,
-               .jim_handler = jim_jtag_newtap,
+               .handler = handle_jtag_newtap,
                .help = "Create a new TAP instance named basename.tap_type, "
                        "and appends it to the scan chain.",
                .usage = "basename tap_type '-irlen' count "
index e1c00b70654f526e14769b4a2711bb30ab33014b..6b492e6ab25613e1ab3d04623ebbd190241e2d0a 100644 (file)
@@ -58,8 +58,15 @@ static const struct command_registration dapdirect_jtag_subcommand_handlers[] =
        {
                .name = "newtap",
                .mode = COMMAND_CONFIG,
-               .jim_handler = jim_jtag_newtap,
-               .help = "declare a new TAP"
+               .handler = handle_jtag_newtap,
+               .help = "declare a new TAP",
+               .usage = "basename tap_type '-irlen' count "
+                       "['-enable'|'-disable'] "
+                       "['-expected_id' number] "
+                       "['-ignore-version'] "
+                       "['-ignore-bypass'] "
+                       "['-ircapture' number] "
+                       "['-mask' number]",
        },
        {
                .name = "init",
@@ -135,8 +142,15 @@ static const struct command_registration dapdirect_swd_subcommand_handlers[] = {
        {
                .name = "newdap",
                .mode = COMMAND_CONFIG,
-               .jim_handler = jim_jtag_newtap,
+               .handler = handle_jtag_newtap,
                .help = "declare a new SWD DAP",
+               .usage = "basename dap_type ['-irlen' count] "
+                       "['-enable'|'-disable'] "
+                       "['-expected_id' number] "
+                       "['-ignore-version'] "
+                       "['-ignore-bypass'] "
+                       "['-ircapture' number] "
+                       "['-mask' number]",
        },
        COMMAND_REGISTRATION_DONE
 };
index 653f91f130ebd3ef941a7e3da8df9d39f59c892f..275a501286b9868ca1f2390deac0b8e03f3cf1c0 100644 (file)
@@ -657,9 +657,16 @@ static const struct command_registration swd_commands[] = {
                 * REVISIT can we verify "just one SWD DAP" here/early?
                 */
                .name = "newdap",
-               .jim_handler = jim_jtag_newtap,
+               .handler = handle_jtag_newtap,
                .mode = COMMAND_CONFIG,
-               .help = "declare a new SWD DAP"
+               .help = "declare a new SWD DAP",
+               .usage = "basename dap_type ['-irlen' count] "
+                       "['-enable'|'-disable'] "
+                       "['-expected_id' number] "
+                       "['-ignore-version'] "
+                       "['-ignore-bypass'] "
+                       "['-ircapture' number] "
+                       "['-mask' number]",
        },
        COMMAND_REGISTRATION_DONE
 };

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)