This is an automated email from Gerrit. Antonio Borneo ([email protected]) just uploaded a new patch set to Gerrit, which you can find at http://openocd.zylin.com/5667
-- gerrit commit 832201a5be698d4315d056b842e401d79c8d89dd Author: Antonio Borneo <[email protected]> Date: Tue May 12 11:52:56 2020 +0200 helper/command: override target only on target prefixed cmds In current code the current target is overridden whenever jim_handler_data is not NULL. This happens not only with target prefixed commands, but also with cti and dap prefixed commands. While this is not causing any run-time issue, by now, the behaviour is tricky and makes the code cryptic. Add a specific field to struct command for the target override so the content of jim_handler_data can be restricted to command specific data only (today only cti and dap). Extend the API register_commands() to specify the presence of either the command data or the override target. The new API makes obsolete calling command_set_handler_data() to set jim_handler_data, so remove it. Change-Id: Icc323faf754b0546a72208f90abd9e68ff2ef52f Signed-off-by: Antonio Borneo <[email protected]> diff --git a/src/helper/command.c b/src/helper/command.c index 5fac406..8b84b44 100644 --- a/src/helper/command.c +++ b/src/helper/command.c @@ -355,8 +355,9 @@ static struct command *register_command(struct command_context *context, return c; } -int register_commands(struct command_context *cmd_ctx, struct command *parent, - const struct command_registration *cmds) +int __register_commands(struct command_context *cmd_ctx, struct command *parent, + const struct command_registration *cmds, void *data, + struct target *override_target) { int retval = ERROR_OK; unsigned i; @@ -370,10 +371,12 @@ int register_commands(struct command_context *cmd_ctx, struct command *parent, retval = ERROR_FAIL; break; } + c->jim_handler_data = data; + c->jim_override_target = override_target; } if (NULL != cr->chain) { struct command *p = c ? : parent; - retval = register_commands(cmd_ctx, p, cr->chain); + retval = __register_commands(cmd_ctx, p, cr->chain, data, override_target); if (ERROR_OK != retval) break; } @@ -425,13 +428,6 @@ static int unregister_command(struct command_context *context, return ERROR_OK; } -void command_set_handler_data(struct command *c, void *p) -{ - c->jim_handler_data = p; - for (struct command *cc = c->children; NULL != cc; cc = cc->next) - command_set_handler_data(cc, p); -} - void command_output_text(struct command_context *context, const char *data) { if (context && context->output_handler && data) @@ -1031,12 +1027,12 @@ static int command_unknown(Jim_Interp *interp, int argc, Jim_Obj *const *argv) * correct work when command_unknown() is re-entered. */ struct target *saved_target_override = cmd_ctx->current_target_override; - if (c->jim_handler_data) - cmd_ctx->current_target_override = c->jim_handler_data; + if (c->jim_override_target) + cmd_ctx->current_target_override = c->jim_override_target; int retval = exec_command(interp, cmd_ctx, c, count, start); - if (c->jim_handler_data) + if (c->jim_override_target) cmd_ctx->current_target_override = saved_target_override; return retval; diff --git a/src/helper/command.h b/src/helper/command.h index 886bde8..eb7b6fc 100644 --- a/src/helper/command.h +++ b/src/helper/command.h @@ -186,11 +186,9 @@ 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 */ + /* Command handlers can use it for any handler specific data */ + struct target *jim_override_target; + /* Used only for target of target-prefixed cmd */ enum command_mode mode; struct command *next; }; @@ -233,6 +231,10 @@ struct command_registration { /** Use this as the last entry in an array of command_registration records. */ #define COMMAND_REGISTRATION_DONE { .name = NULL, .chain = NULL } +int __register_commands(struct command_context *cmd_ctx, struct command *parent, + const struct command_registration *cmds, void *data, + struct target *override_target); + /** * Register one or more commands in the specified context, as children * of @c parent (or top-level commends, if NULL). In a registration's @@ -248,8 +250,53 @@ struct command_registration { * NULL for all fields. * @returns ERROR_OK on success; ERROR_FAIL if any registration fails. */ -int register_commands(struct command_context *cmd_ctx, struct command *parent, - const struct command_registration *cmds); +static inline int register_commands(struct command_context *cmd_ctx, struct command *parent, + const struct command_registration *cmds) +{ + return __register_commands(cmd_ctx, parent, cmds, NULL, NULL); +} + +/** + * Register one or more commands, as register_commands(), plus specify + * that command should override the current target + * + * @param cmd_ctx The command_context in which to register the command. + * @param parent Register this command as a child of this, or NULL to + * register a top-level command. + * @param cmds Pointer to an array of command_registration records that + * contains the desired command parameters. The last record must have + * NULL for all fields. + * @param target The target that has to override current target. + * @returns ERROR_OK on success; ERROR_FAIL if any registration fails. + */ +static inline int register_commands_override_target(struct command_context *cmd_ctx, + struct command *parent, const struct command_registration *cmds, + struct target *target) +{ + return __register_commands(cmd_ctx, parent, cmds, NULL, target); +} + +/** + * Register one or more commands, as register_commands(), plus specify + * a pointer to command private data that would be accessible through + * the macro CMD_DATA. The private data will not be freed when command + * is unregistered. + * + * @param cmd_ctx The command_context in which to register the command. + * @param parent Register this command as a child of this, or NULL to + * register a top-level command. + * @param cmds Pointer to an array of command_registration records that + * contains the desired command parameters. The last record must have + * NULL for all fields. + * @param data The command private data. + * @returns ERROR_OK on success; ERROR_FAIL if any registration fails. + */ +static inline int register_commands_with_data(struct command_context *cmd_ctx, + struct command *parent, const struct command_registration *cmds, + void *data) +{ + return __register_commands(cmd_ctx, parent, cmds, data, NULL); +} /** * Unregisters all commands from the specfied context. @@ -263,16 +310,6 @@ int unregister_all_commands(struct command_context *cmd_ctx, struct command *command_find_in_context(struct command_context *cmd_ctx, const char *name); -/** - * Update the private command data field for a command and all descendents. - * This is used when creating a new heirarchy of commands that depends - * on obtaining a dynamically created context. The value will be available - * in command handlers by using the CMD_DATA macro. - * @param c The command (group) whose data pointer(s) will be updated. - * @param p The new data pointer to use for the command or its descendents. - */ -void command_set_handler_data(struct command *c, void *p); - void command_set_output_handler(struct command_context *context, command_output_handler_t output_handler, void *priv); diff --git a/src/target/arm_cti.c b/src/target/arm_cti.c index 579bacb..67bfe70 100644 --- a/src/target/arm_cti.c +++ b/src/target/arm_cti.c @@ -566,17 +566,13 @@ static int cti_create(Jim_GetOptInfo *goi) }, COMMAND_REGISTRATION_DONE }; - e = register_commands(cmd_ctx, NULL, cti_commands); + e = register_commands_with_data(cmd_ctx, NULL, cti_commands, cti); if (ERROR_OK != e) return JIM_ERR; - struct command *c = command_find_in_context(cmd_ctx, cp); - assert(c); - command_set_handler_data(c, cti); - list_add_tail(&cti->lh, &all_cti); - return (ERROR_OK == e) ? JIM_OK : JIM_ERR; + return JIM_OK; } static int jim_cti_create(Jim_Interp *interp, int argc, Jim_Obj *const *argv) diff --git a/src/target/arm_dap.c b/src/target/arm_dap.c index 56442f1..a9277e7 100644 --- a/src/target/arm_dap.c +++ b/src/target/arm_dap.c @@ -265,17 +265,13 @@ static int dap_create(Jim_GetOptInfo *goi) if (transport_is_hla()) dap_commands[0].chain = NULL; - e = register_commands(cmd_ctx, NULL, dap_commands); + e = register_commands_with_data(cmd_ctx, NULL, dap_commands, dap); if (ERROR_OK != e) return JIM_ERR; - struct command *c = command_find_in_context(cmd_ctx, cp); - assert(c); - command_set_handler_data(c, dap); - list_add_tail(&dap->lh, &all_dap); - return (ERROR_OK == e) ? JIM_OK : JIM_ERR; + return JIM_OK; } static int jim_dap_create(Jim_Interp *interp, int argc, Jim_Obj *const *argv) diff --git a/src/target/target.c b/src/target/target.c index a43c1a5..aaa2851 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -5629,14 +5629,7 @@ static int target_create(Jim_GetOptInfo *goi) }, COMMAND_REGISTRATION_DONE }; - e = register_commands(cmd_ctx, NULL, target_commands); - if (ERROR_OK != e) - return JIM_ERR; - - struct command *c = command_find_in_context(cmd_ctx, cp); - assert(c); - command_set_handler_data(c, target); - + e = register_commands_override_target(cmd_ctx, NULL, target_commands, target); return (ERROR_OK == e) ? JIM_OK : JIM_ERR; } -- _______________________________________________ OpenOCD-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openocd-devel
