helper/command: always pass struct command as jim private data

While registering a new command, jim accepts a pointer to command's
private data that will be accessible during the command execution.

Today openocd is not consistent and passes different private data
depending on the command, and then even overwrites it:
- "simple" commands (.handler) are registered with their own
  struct command pointer as command private data;
- "native" commands (.jim_handler) at root level are registered
  with NULL command private data;
- "native" commands (.jim_handler) not at root level are
  registered with the struct command pointer of their root command
  as command private data but, when executed, the command private
  data is overwritten by the value in field jim_handler_data taken
  from their struct command.

Uniform the usage of command private data by always set it to the
struct command pointer while registering the new commands.
Note: for multi-word commands only the root command is registered,
so command private data will be set to the struct command of the
root command. This will change later in this series when the full-
name of the command will be registered.

Don't overwrite the command private data, but let the commands that
needs jim_handler_data to get it directly through struct command.

For sake of uniformity, let function command_set_handler_data() to
set the field jim_handler_data also for "group" commands, even if
such value will not be used.

Now Jim_CmdPrivData() always returns a struct command pointer, so
wrap it in the inline function jim_to_command() to gain compile
time check on the returned type.
While there, uniform the code to use the macro Jim_CmdPrivData()
to access the command's private data pointer.

Change-Id: Idba16242ba1f6769341b4030a49cdf35a5278695
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: http://openocd.zylin.com/5664
Tested-by: jenkins
Reviewed-by: Oleksij Rempel <linux@rempel-privat.de>

diff --git a/src/helper/command.c b/src/helper/command.c
index e2726f258fad682456eb05c4e78b4fcc20298b99..b44e4667f9dadee286abb6e2b8e9bf3ab6676821 100644 (file)
--- a/src/helper/command.c
+++ b/src/helper/command.c
@@ -248,7 +248,7 @@ static int script_command(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
 {
        /* the private data is stashed in the interp structure */
 
-       struct command *c = interp->cmdPrivData;
+       struct command *c = jim_to_command(interp);
        assert(c);
        script_debug(interp, argc, argv);
        return script_command_run(interp, argc, argv, c);
@@ -418,7 +418,7 @@ static struct command *register_command(struct command_context *context,
        int retval = JIM_OK;
        if (NULL != cr->jim_handler && NULL == parent) {
                retval = Jim_CreateCommand(context->interp, cr->name,
-                               cr->jim_handler, NULL, NULL);
+                               cr->jim_handler, c, NULL);
        } else if (NULL != cr->handler || NULL != parent)
                retval = register_command_handler(context, command_root(c));
 
@@ -501,8 +501,7 @@ static int unregister_command(struct command_context *context,
 
 void command_set_handler_data(struct command *c, void *p)
 {
-       if (NULL != c->handler || NULL != c->jim_handler)
-               c->jim_handler_data = p;
+       c->jim_handler_data = p;
        for (struct command *cc = c->children; NULL != cc; cc = cc->next)
                command_set_handler_data(cc, p);
 }
@@ -1085,7 +1084,6 @@ static int command_unknown(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
                if (!command_can_run(cmd_ctx, c))
                        return JIM_ERR;
 
-               interp->cmdPrivData = c->jim_handler_data;
                return (*c->jim_handler)(interp, count, start);
        }
 

diff --git a/src/helper/command.h b/src/helper/command.h
index b0c84bb43d629a3226e1c4495d1fdadb543b392e..cb088d74388aefa2194bc23ca9dffacdf5fde4ef 100644 (file)
--- a/src/helper/command.h
+++ b/src/helper/command.h
@@ -195,6 +195,15 @@ struct command {
        struct command *next;
 };
 
+/*
+ * Return the struct command pointer kept in private data
+ * Used to enforce check on data type
+ */
+static inline struct command *jim_to_command(Jim_Interp *interp)
+{
+       return Jim_CmdPrivData(interp);
+}
+
 /*
  * Commands should be registered by filling in one or more of these
  * structures and passing them to [un]register_commands().

diff --git a/src/target/arm_tpiu_swo.c b/src/target/arm_tpiu_swo.c
index 2da52e89260d8fcd6a6b97e6ea5ca071e3bafb1e..543b4f00849c8aefb2e374a7c4854b4a8f16a772 100644 (file)
--- a/src/target/arm_tpiu_swo.c
+++ b/src/target/arm_tpiu_swo.c
@@ -559,7 +559,8 @@ static int jim_arm_tpiu_swo_configure(Jim_Interp *interp, int argc, Jim_Obj * co
                        "missing: -option ...");
                return JIM_ERR;
        }
-       struct arm_tpiu_swo_object *obj = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct arm_tpiu_swo_object *obj = c->jim_handler_data;
        return arm_tpiu_swo_configure(&goi, obj);
 }
 
@@ -583,7 +584,8 @@ static int wrap_read_u32(struct target *target, struct adiv5_ap *tpiu_ap,
 
 static int jim_arm_tpiu_swo_enable(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
 {
-       struct arm_tpiu_swo_object *obj = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct arm_tpiu_swo_object *obj = c->jim_handler_data;
        struct command_context *cmd_ctx = current_command_context(interp);
        struct adiv5_ap *tpiu_ap = dap_ap(obj->spot.dap, obj->spot.ap_num);
        uint32_t value;
@@ -786,7 +788,8 @@ error_exit:
 
 static int jim_arm_tpiu_swo_disable(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
 {
-       struct arm_tpiu_swo_object *obj = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct arm_tpiu_swo_object *obj = c->jim_handler_data;
 
        if (argc != 1) {
                Jim_WrongNumArgs(interp, 1, argv, "Too many parameters");

diff --git a/src/target/nds32_cmd.c b/src/target/nds32_cmd.c
index 1684ea8824483e178d91ebb114096758c43b0561..89da845d3f8ba82c21b7156e2d9ffe86ba11ae86 100644 (file)
--- a/src/target/nds32_cmd.c
+++ b/src/target/nds32_cmd.c
@@ -722,7 +722,8 @@ static int jim_nds32_bulk_write(Jim_Interp *interp, int argc, Jim_Obj * const *a
                return JIM_ERR;
        }
 
-       struct target *target = Jim_CmdPrivData(goi.interp);
+       struct command *c = jim_to_command(goi.interp);
+       struct target *target = c->jim_handler_data;
        int result;
 
        result = target_write_buffer(target, address, count * 4, (const uint8_t *)data);
@@ -751,7 +752,8 @@ static int jim_nds32_multi_write(Jim_Interp *interp, int argc, Jim_Obj * const *
        if (e != JIM_OK)
                return e;
 
-       struct target *target = Jim_CmdPrivData(goi.interp);
+       struct command *c = jim_to_command(goi.interp);
+       struct target *target = c->jim_handler_data;
        struct aice_port_s *aice = target_to_aice(target);
        int result;
        uint32_t address;
@@ -812,7 +814,8 @@ static int jim_nds32_bulk_read(Jim_Interp *interp, int argc, Jim_Obj * const *ar
        if (goi.argc != 0)
                return JIM_ERR;
 
-       struct target *target = Jim_CmdPrivData(goi.interp);
+       struct command *c = jim_to_command(goi.interp);
+       struct target *target = c->jim_handler_data;
        uint32_t *data = malloc(count * sizeof(uint32_t));
        int result;
        result = target_read_buffer(target, address, count * 4, (uint8_t *)data);
@@ -863,7 +866,8 @@ static int jim_nds32_read_edm_sr(Jim_Interp *interp, int argc, Jim_Obj * const *
        else
                return ERROR_FAIL;
 
-       struct target *target = Jim_CmdPrivData(goi.interp);
+       struct command *c = jim_to_command(goi.interp);
+       struct target *target = c->jim_handler_data;
        struct aice_port_s *aice = target_to_aice(target);
        char data_str[11];
 
@@ -911,7 +915,8 @@ static int jim_nds32_write_edm_sr(Jim_Interp *interp, int argc, Jim_Obj * const
        else
                return ERROR_FAIL;
 
-       struct target *target = Jim_CmdPrivData(goi.interp);
+       struct command *c = jim_to_command(goi.interp);
+       struct target *target = c->jim_handler_data;
        struct aice_port_s *aice = target_to_aice(target);
 
        aice_write_debug_reg(aice, edm_sr_number, value);

diff --git a/src/target/target.c b/src/target/target.c
index 200368b133000c4e40efb514983af98d9b785d2e..e481d526cd24ee905215c667ff5fe0692166c81d 100644 (file)
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -5206,21 +5206,24 @@ static int jim_target_configure(Jim_Interp *interp, int argc, Jim_Obj * const *a
                                 "missing: -option ...");
                return JIM_ERR;
        }
-       struct target *target = Jim_CmdPrivData(goi.interp);
+       struct command *c = jim_to_command(goi.interp);
+       struct target *target = c->jim_handler_data;
        return target_configure(&goi, target);
 }
 
 static int jim_target_mem2array(Jim_Interp *interp,
                int argc, Jim_Obj *const *argv)
 {
-       struct target *target = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct target *target = c->jim_handler_data;
        return target_mem2array(interp, target, argc - 1, argv + 1);
 }
 
 static int jim_target_array2mem(Jim_Interp *interp,
                int argc, Jim_Obj *const *argv)
 {
-       struct target *target = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct target *target = c->jim_handler_data;
        return target_array2mem(interp, target, argc - 1, argv + 1);
 }
 
@@ -5252,7 +5255,8 @@ static int jim_target_examine(Jim_Interp *interp, int argc, Jim_Obj *const *argv
                allow_defer = true;
        }
 
-       struct target *target = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct target *target = c->jim_handler_data;
        if (!target->tap->enabled)
                return jim_target_tap_disabled(interp);
 
@@ -5270,7 +5274,8 @@ static int jim_target_examine(Jim_Interp *interp, int argc, Jim_Obj *const *argv
 
 static int jim_target_was_examined(Jim_Interp *interp, int argc, Jim_Obj * const *argv)
 {
-       struct target *target = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct target *target = c->jim_handler_data;
 
        Jim_SetResultBool(interp, target_was_examined(target));
        return JIM_OK;
@@ -5278,7 +5283,8 @@ static int jim_target_was_examined(Jim_Interp *interp, int argc, Jim_Obj * const
 
 static int jim_target_examine_deferred(Jim_Interp *interp, int argc, Jim_Obj * const *argv)
 {
-       struct target *target = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct target *target = c->jim_handler_data;
 
        Jim_SetResultBool(interp, target->defer_examine);
        return JIM_OK;
@@ -5290,7 +5296,8 @@ static int jim_target_halt_gdb(Jim_Interp *interp, int argc, Jim_Obj *const *arg
                Jim_WrongNumArgs(interp, 1, argv, "[no parameters]");
                return JIM_ERR;
        }
-       struct target *target = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct target *target = c->jim_handler_data;
 
        if (target_call_event_callbacks(target, TARGET_EVENT_GDB_HALT) != ERROR_OK)
                return JIM_ERR;
@@ -5304,7 +5311,8 @@ static int jim_target_poll(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
                Jim_WrongNumArgs(interp, 1, argv, "[no parameters]");
                return JIM_ERR;
        }
-       struct target *target = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct target *target = c->jim_handler_data;
        if (!target->tap->enabled)
                return jim_target_tap_disabled(interp);
 
@@ -5341,7 +5349,8 @@ static int jim_target_reset(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
        if (e != JIM_OK)
                return e;
 
-       struct target *target = Jim_CmdPrivData(goi.interp);
+       struct command *c = jim_to_command(goi.interp);
+       struct target *target = c->jim_handler_data;
        if (!target->tap->enabled)
                return jim_target_tap_disabled(interp);
 
@@ -5374,7 +5383,8 @@ static int jim_target_halt(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
                Jim_WrongNumArgs(interp, 1, argv, "[no parameters]");
                return JIM_ERR;
        }
-       struct target *target = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct target *target = c->jim_handler_data;
        if (!target->tap->enabled)
                return jim_target_tap_disabled(interp);
        int e = target->type->halt(target);
@@ -5404,7 +5414,8 @@ static int jim_target_wait_state(Jim_Interp *interp, int argc, Jim_Obj *const *a
        e = Jim_GetOpt_Wide(&goi, &a);
        if (e != JIM_OK)
                return e;
-       struct target *target = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct target *target = c->jim_handler_data;
        if (!target->tap->enabled)
                return jim_target_tap_disabled(interp);
 
@@ -5448,7 +5459,8 @@ static int jim_target_current_state(Jim_Interp *interp, int argc, Jim_Obj *const
                Jim_WrongNumArgs(interp, 1, argv, "[no parameters]");
                return JIM_ERR;
        }
-       struct target *target = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct target *target = c->jim_handler_data;
        Jim_SetResultString(interp, target_state_name(target), -1);
        return JIM_OK;
 }
@@ -5467,7 +5479,8 @@ static int jim_target_invoke_event(Jim_Interp *interp, int argc, Jim_Obj *const
                Jim_GetOpt_NvpUnknown(&goi, nvp_target_event, 1);
                return e;
        }
-       struct target *target = Jim_CmdPrivData(interp);
+       struct command *c = jim_to_command(interp);
+       struct target *target = c->jim_handler_data;
        target_handle_event(target, n->value);
        return JIM_OK;
 }