target: use correct target in target-prefixed commands and event handlers

This change contains an alternative to Matthias Welwarsky's #4130
(target-prefixed commands) and to #4293 (event handlers).

get_current_target() must retrieve the target associated to the current
command. If no target associated, the current target of the command
context is used as a fallback.

Many Tcl event handlers work with the current target as if it were
the target issuing the event.

current_target in command_context is a number and has to be converted
to a pointer in every get_current_target() call.

The solution:
- Replace current_target in command_context by a target pointer
- Add another target pointer current_target_override
- get_current_target() returns current_target_override if set, otherwise
	current_target
- Save, set and restore current_target_override to the current prefix
	in run_command()
- Save, set and restore current_target_override to the event invoking
	target in target_handle_event()

While on it use calloc when allocating a new command_context.

Change-Id: I9a82102e94dcac063743834a1d28da861b2e74ea
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Suggested-by: Matthias Welwarsky <matthias.welwarsky@sysgo.com>
Reviewed-on: http://openocd.zylin.com/4295
Tested-by: jenkins
Reviewed-by: Matthias Welwarsky <matthias@welwarsky.de>

diff --git a/doc/openocd.texi b/doc/openocd.texi
index a6f220f46d6e46ab9d3bf4767f15de6cc9e0c4aa..9126003b8be5c02725810cfff4dca7d56d4dbe3e 100644 (file)
--- a/doc/openocd.texi
+++ b/doc/openocd.texi
@@ -4217,6 +4217,9 @@ Calling this twice with two different event names assigns
 two different handlers, but calling it twice with the
 same event name assigns only one handler.
 
+Current target is temporarily overridden to the event issuing target
+before handler code starts and switched back after handler is done.
+
 @item @code{-work-area-backup} (@option{0}|@option{1}) -- says
 whether the work area gets backed up; by default,
 @emph{it is not backed up.}

diff --git a/src/helper/command.c b/src/helper/command.c
index cbd52fbf13fe61a4d41a145454b61410e4c3dee8..f8b731e4a0a3538eccab9b03a46503c96d7b58c6 100644 (file)
--- a/src/helper/command.c
+++ b/src/helper/command.c
@@ -608,7 +608,23 @@ static int run_command(struct command_context *context,
                .argc = num_words - 1,
                .argv = words + 1,
        };
+       /* Black magic of overridden current target:
+        * If the command we are going to handle has a target prefix,
+        * override the current target temporarily for the time
+        * of processing the command.
+        * current_target_override is used also for event handlers
+        * therefore we prevent touching it if command has no prefix.
+        * Previous override is saved and restored back to ensure
+        * correct work when run_command() is re-entered. */
+       struct target *saved_target_override = context->current_target_override;
+       if (c->jim_handler_data)
+               context->current_target_override = c->jim_handler_data;
+
        int retval = c->handler(&cmd);
+
+       if (c->jim_handler_data)
+               context->current_target_override = saved_target_override;
+
        if (retval == ERROR_COMMAND_SYNTAX_ERROR) {
                /* Print help for command */
                char *full_name = command_name(c, ' ');
@@ -643,6 +659,8 @@ int command_run_line(struct command_context *context, char *line)
         * happen when the Jim Tcl interpreter is provided by eCos for
         * instance.
         */
+       context->current_target_override = NULL;
+
        Jim_Interp *interp = context->interp;
        Jim_DeleteAssocData(interp, "context");
        retcode = Jim_SetAssocData(interp, "context", NULL, context);
@@ -1269,14 +1287,10 @@ static const struct command_registration command_builtin_handlers[] = {
 
 struct command_context *command_init(const char *startup_tcl, Jim_Interp *interp)
 {
-       struct command_context *context = malloc(sizeof(struct command_context));
+       struct command_context *context = calloc(1, sizeof(struct command_context));
        const char *HostOs;
 
        context->mode = COMMAND_EXEC;
-       context->commands = NULL;
-       context->current_target = 0;
-       context->output_handler = NULL;
-       context->output_handler_priv = NULL;
 
        /* Create a jim interpreter if we were not handed one */
        if (interp == NULL) {

diff --git a/src/helper/command.h b/src/helper/command.h
index f696ab823bc16811623ee4b3e34325c83850f91a..1a26c069956af15e6d3302b444ccc5ff8d8e4aca 100644 (file)
--- a/src/helper/command.h
+++ b/src/helper/command.h
@@ -49,7 +49,15 @@ struct command_context {
        Jim_Interp *interp;
        enum command_mode mode;
        struct command *commands;
-       int current_target;
+       struct target *current_target;
+               /* The target set by 'targets xx' command or the latest created */
+       struct target *current_target_override;
+               /* If set overrides current_target
+                * It happens during processing of
+                *      1) a target prefixed command
+                *      2) an event handler
+                * Pay attention to reentrancy when setting override.
+                */
        command_output_handler_t output_handler;
        void *output_handler_priv;
 };
@@ -168,6 +176,11 @@ struct command {
        command_handler_t handler;
        Jim_CmdProc *jim_handler;
        void *jim_handler_data;
+               /* Currently used only for target of target-prefixed cmd.
+                * Native OpenOCD commands use jim_handler_data exclusively
+                * as a target override.
+                * Jim handlers outside of target cmd tree can use
+                * jim_handler_data for any handler specific data */
        enum command_mode mode;
        struct command *next;
 };

diff --git a/src/server/tcl_server.c b/src/server/tcl_server.c
index 0077339f23c915abd0c92e8d52a72bf99b64f483..7c40f7dce9c49908e09b234183137288f6ebc2ae 100644 (file)
--- a/src/server/tcl_server.c
+++ b/src/server/tcl_server.c
@@ -157,7 +157,7 @@ static int tcl_new_connection(struct connection *connection)
 
        connection->priv = tclc;
 
-       struct target *target = get_target_by_num(connection->cmd_ctx->current_target);
+       struct target *target = get_current_target(connection->cmd_ctx);
        if (target != NULL)
                tclc->tc_laststate = target->state;
 

diff --git a/src/target/target.c b/src/target/target.c
index 52376101577cef734a10c7c6ad62a23b1ccdca45..32000c0474b3c2a7c0466632db288912a0a2f823 100644 (file)
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -510,7 +510,9 @@ struct target *get_target_by_num(int num)
 
 struct target *get_current_target(struct command_context *cmd_ctx)
 {
-       struct target *target = get_target_by_num(cmd_ctx->current_target);
+       struct target *target = cmd_ctx->current_target_override
+               ? cmd_ctx->current_target_override
+               : cmd_ctx->current_target;
 
        if (target == NULL) {
                LOG_ERROR("BUG: current_target out of bounds");
@@ -2505,7 +2507,10 @@ static int find_target(struct command_context *cmd_ctx, const char *name)
                return ERROR_FAIL;
        }
 
-       cmd_ctx->current_target = target->target_number;
+       cmd_ctx->current_target = target;
+       if (cmd_ctx->current_target_override)
+               cmd_ctx->current_target_override = target;
+
        return ERROR_OK;
 }
 
@@ -2533,7 +2538,7 @@ COMMAND_HANDLER(handle_targets_command)
                else
                        state = "tap-disabled";
 
-               if (CMD_CTX->current_target == target->target_number)
+               if (CMD_CTX->current_target == target)
                        marker = '*';
 
                /* keep columns lined up to match the headers above */
@@ -4441,17 +4446,28 @@ void target_handle_event(struct target *target, enum target_event e)
 
        for (teap = target->event_action; teap != NULL; teap = teap->next) {
                if (teap->event == e) {
-                       LOG_DEBUG("target: (%d) %s (%s) event: %d (%s) action: %s",
+                       LOG_DEBUG("target(%d): %s (%s) event: %d (%s) action: %s",
                                           target->target_number,
                                           target_name(target),
                                           target_type_name(target),
                                           e,
                                           Jim_Nvp_value2name_simple(nvp_target_event, e)->name,
                                           Jim_GetString(teap->body, NULL));
+
+                       /* Override current target by the target an event
+                        * is issued from (lot of scripts need it).
+                        * Return back to previous override as soon
+                        * as the handler processing is done */
+                       struct command_context *cmd_ctx = current_command_context(teap->interp);
+                       struct target *saved_target_override = cmd_ctx->current_target_override;
+                       cmd_ctx->current_target_override = target;
+
                        if (Jim_EvalObj(teap->interp, teap->body) != JIM_OK) {
                                Jim_MakeErrorMessage(teap->interp);
                                command_print(NULL, "%s\n", Jim_GetString(Jim_GetResult(teap->interp), NULL));
                        }
+
+                       cmd_ctx->current_target_override = saved_target_override;
                }
        }
 }
@@ -5529,7 +5545,7 @@ static int target_create(Jim_GetOptInfo *goi)
        target = calloc(1, sizeof(struct target));
        /* set target number */
        target->target_number = new_target_number();
-       cmd_ctx->current_target = target->target_number;
+       cmd_ctx->current_target = target;
 
        /* allocate memory for each unique target type */
        target->type = calloc(1, sizeof(struct target_type));