From bb9d9c60264a905926e0d15f84842858d0de80b7 Mon Sep 17 00:00:00 2001 From: Tomas Vanek Date: Tue, 21 Nov 2017 22:57:39 +0100 Subject: [PATCH] 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 Suggested-by: Matthias Welwarsky Reviewed-on: http://openocd.zylin.com/4295 Tested-by: jenkins Reviewed-by: Matthias Welwarsky --- doc/openocd.texi | 3 +++ src/helper/command.c | 24 +++++++++++++++++++----- src/helper/command.h | 15 ++++++++++++++- src/server/tcl_server.c | 2 +- src/target/target.c | 26 +++++++++++++++++++++----- 5 files changed, 58 insertions(+), 12 deletions(-) diff --git a/doc/openocd.texi b/doc/openocd.texi index a6f220f4..9126003b 100644 --- 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 cbd52fbf..f8b731e4 100644 --- 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 f696ab82..1a26c069 100644 --- 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 0077339f..7c40f7dc 100644 --- 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 52376101..32000c04 100644 --- 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)); -- 2.39.5