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>
riscv-compliance-dev
Tomas Vanek 2017-11-21 22:57:39 +01:00 committed by Matthias Welwarsky
parent eaeb4191e5
commit bb9d9c6026
5 changed files with 58 additions and 12 deletions

View File

@ -4217,6 +4217,9 @@ Calling this twice with two different event names assigns
two different handlers, but calling it twice with the two different handlers, but calling it twice with the
same event name assigns only one handler. 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 @item @code{-work-area-backup} (@option{0}|@option{1}) -- says
whether the work area gets backed up; by default, whether the work area gets backed up; by default,
@emph{it is not backed up.} @emph{it is not backed up.}

View File

@ -608,7 +608,23 @@ static int run_command(struct command_context *context,
.argc = num_words - 1, .argc = num_words - 1,
.argv = 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); int retval = c->handler(&cmd);
if (c->jim_handler_data)
context->current_target_override = saved_target_override;
if (retval == ERROR_COMMAND_SYNTAX_ERROR) { if (retval == ERROR_COMMAND_SYNTAX_ERROR) {
/* Print help for command */ /* Print help for command */
char *full_name = command_name(c, ' '); 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 * happen when the Jim Tcl interpreter is provided by eCos for
* instance. * instance.
*/ */
context->current_target_override = NULL;
Jim_Interp *interp = context->interp; Jim_Interp *interp = context->interp;
Jim_DeleteAssocData(interp, "context"); Jim_DeleteAssocData(interp, "context");
retcode = Jim_SetAssocData(interp, "context", NULL, 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 *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; const char *HostOs;
context->mode = COMMAND_EXEC; 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 */ /* Create a jim interpreter if we were not handed one */
if (interp == NULL) { if (interp == NULL) {

View File

@ -49,7 +49,15 @@ struct command_context {
Jim_Interp *interp; Jim_Interp *interp;
enum command_mode mode; enum command_mode mode;
struct command *commands; 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; command_output_handler_t output_handler;
void *output_handler_priv; void *output_handler_priv;
}; };
@ -168,6 +176,11 @@ struct command {
command_handler_t handler; command_handler_t handler;
Jim_CmdProc *jim_handler; Jim_CmdProc *jim_handler;
void *jim_handler_data; 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; enum command_mode mode;
struct command *next; struct command *next;
}; };

View File

@ -157,7 +157,7 @@ static int tcl_new_connection(struct connection *connection)
connection->priv = tclc; 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) if (target != NULL)
tclc->tc_laststate = target->state; tclc->tc_laststate = target->state;

View File

@ -510,7 +510,9 @@ struct target *get_target_by_num(int num)
struct target *get_current_target(struct command_context *cmd_ctx) 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) { if (target == NULL) {
LOG_ERROR("BUG: current_target out of bounds"); 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; 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; return ERROR_OK;
} }
@ -2533,7 +2538,7 @@ COMMAND_HANDLER(handle_targets_command)
else else
state = "tap-disabled"; state = "tap-disabled";
if (CMD_CTX->current_target == target->target_number) if (CMD_CTX->current_target == target)
marker = '*'; marker = '*';
/* keep columns lined up to match the headers above */ /* 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) { for (teap = target->event_action; teap != NULL; teap = teap->next) {
if (teap->event == e) { 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->target_number,
target_name(target), target_name(target),
target_type_name(target), target_type_name(target),
e, e,
Jim_Nvp_value2name_simple(nvp_target_event, e)->name, Jim_Nvp_value2name_simple(nvp_target_event, e)->name,
Jim_GetString(teap->body, NULL)); 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) { if (Jim_EvalObj(teap->interp, teap->body) != JIM_OK) {
Jim_MakeErrorMessage(teap->interp); Jim_MakeErrorMessage(teap->interp);
command_print(NULL, "%s\n", Jim_GetString(Jim_GetResult(teap->interp), NULL)); 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)); target = calloc(1, sizeof(struct target));
/* set target number */ /* set target number */
target->target_number = new_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 */ /* allocate memory for each unique target type */
target->type = calloc(1, sizeof(struct target_type)); target->type = calloc(1, sizeof(struct target_type));