From: OpenOCD-Gerrit <ope...@us...> - 2021-04-18 14:34:29
|
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "Main OpenOCD repository". The branch, master has been updated via 41c95aa4ea1506a951dad0147f6cd4b8d7043358 (commit) via 428938993742f4f961cdc948593d9553f721c321 (commit) via 7cd679a2de697065bbd36cde9042516ccf20f0f1 (commit) via 462323f123ede2c8da59cb4acb772dc4f98dee79 (commit) from a510c8e23c23b7a888623faf0c4a9982489ddf83 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- commit 41c95aa4ea1506a951dad0147f6cd4b8d7043358 Author: Antonio Borneo <bor...@gm...> Date: Sun May 10 19:35:56 2020 +0200 helper/command: pass command prefix to command registration Replace the "struct command *parent" parameter with a string that contains the command prefix. This abstracts the openocd code from the knowledge of the tree of struct command. This also makes unused the function command_find_in_context(), so remove it. Change-Id: I598d60719cfdc1811ee6f6edfff8a116f82c7ed6 Signed-off-by: Antonio Borneo <bor...@gm...> Reviewed-on: http://openocd.zylin.com/5668 Tested-by: jenkins Reviewed-by: Oleksij Rempel <li...@re...> diff --git a/src/flash/nand/tcl.c b/src/flash/nand/tcl.c index ca8b9dad4..9e0ca41ac 100644 --- a/src/flash/nand/tcl.c +++ b/src/flash/nand/tcl.c @@ -479,8 +479,8 @@ static int nand_init(struct command_context *cmd_ctx) { if (!nand_devices) return ERROR_OK; - struct command *parent = command_find_in_context(cmd_ctx, "nand"); - return register_commands(cmd_ctx, parent, nand_exec_command_handlers); + + return register_commands(cmd_ctx, "nand", nand_exec_command_handlers); } COMMAND_HANDLER(handle_nand_init_command) diff --git a/src/flash/nor/esirisc_flash.c b/src/flash/nor/esirisc_flash.c index 88f00bcca..24e811704 100644 --- a/src/flash/nor/esirisc_flash.c +++ b/src/flash/nor/esirisc_flash.c @@ -109,7 +109,6 @@ static const struct command_registration esirisc_flash_command_handlers[]; FLASH_BANK_COMMAND_HANDLER(esirisc_flash_bank_command) { struct esirisc_flash_bank *esirisc_info; - struct command *esirisc_cmd; if (CMD_ARGC < 9) return ERROR_COMMAND_SYNTAX_ERROR; @@ -123,8 +122,7 @@ FLASH_BANK_COMMAND_HANDLER(esirisc_flash_bank_command) bank->driver_priv = esirisc_info; /* register commands using existing esirisc context */ - esirisc_cmd = command_find_in_context(CMD_CTX, "esirisc"); - register_commands(CMD_CTX, esirisc_cmd, esirisc_flash_command_handlers); + register_commands(CMD_CTX, "esirisc", esirisc_flash_command_handlers); return ERROR_OK; } diff --git a/src/flash/nor/tcl.c b/src/flash/nor/tcl.c index 66b9a4cb6..3f737aca3 100644 --- a/src/flash/nor/tcl.c +++ b/src/flash/nor/tcl.c @@ -1248,8 +1248,7 @@ static int flash_init_drivers(struct command_context *cmd_ctx) if (!flash_bank_list()) return ERROR_OK; - struct command *parent = command_find_in_context(cmd_ctx, "flash"); - return register_commands(cmd_ctx, parent, flash_exec_command_handlers); + return register_commands(cmd_ctx, "flash", flash_exec_command_handlers); } COMMAND_HANDLER(handle_flash_bank_command) diff --git a/src/helper/command.c b/src/helper/command.c index 89e217382..114d07328 100644 --- a/src/helper/command.c +++ b/src/helper/command.c @@ -244,12 +244,6 @@ static struct command *command_find(struct command *head, const char *name) return NULL; } -struct command *command_find_in_context(struct command_context *cmd_ctx, - const char *name) -{ - return command_find(cmd_ctx->commands, name); -} - /** * Add the command into the linked list, sorted by name. * @param head Address to head of command list pointer, which may be @@ -391,7 +385,7 @@ static struct command *register_command(struct command_context *context, return c; } -int __register_commands(struct command_context *cmd_ctx, struct command *parent, +static int ___register_commands(struct command_context *cmd_ctx, struct command *parent, const struct command_registration *cmds, void *data, struct target *override_target) { @@ -412,7 +406,7 @@ int __register_commands(struct command_context *cmd_ctx, struct command *parent, } if (NULL != cr->chain) { struct command *p = c ? : parent; - retval = __register_commands(cmd_ctx, p, cr->chain, data, override_target); + retval = ___register_commands(cmd_ctx, p, cr->chain, data, override_target); if (ERROR_OK != retval) break; } @@ -424,6 +418,18 @@ int __register_commands(struct command_context *cmd_ctx, struct command *parent, return retval; } +int __register_commands(struct command_context *cmd_ctx, const char *cmd_prefix, + const struct command_registration *cmds, void *data, + struct target *override_target) +{ + struct command *parent = NULL; + + if (cmd_prefix) + parent = command_find(cmd_ctx->commands, cmd_prefix); + + return ___register_commands(cmd_ctx, parent, cmds, data, override_target); +} + int unregister_all_commands(struct command_context *context, struct command *parent) { diff --git a/src/helper/command.h b/src/helper/command.h index 871c064d3..9a04e9fa1 100644 --- a/src/helper/command.h +++ b/src/helper/command.h @@ -240,7 +240,7 @@ 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, +int __register_commands(struct command_context *cmd_ctx, const char *cmd_prefix, const struct command_registration *cmds, void *data, struct target *override_target); @@ -252,17 +252,17 @@ int __register_commands(struct command_context *cmd_ctx, struct command *parent, * Otherwise, the chained commands are added as children of the command. * * @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 + * @param cmd_prefix 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. * @returns ERROR_OK on success; ERROR_FAIL if any registration fails. */ -static inline int register_commands(struct command_context *cmd_ctx, struct command *parent, +static inline int register_commands(struct command_context *cmd_ctx, const char *cmd_prefix, const struct command_registration *cmds) { - return __register_commands(cmd_ctx, parent, cmds, NULL, NULL); + return __register_commands(cmd_ctx, cmd_prefix, cmds, NULL, NULL); } /** @@ -270,7 +270,7 @@ static inline int register_commands(struct command_context *cmd_ctx, struct comm * 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 + * @param cmd_prefix 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 @@ -279,10 +279,10 @@ static inline int register_commands(struct command_context *cmd_ctx, struct comm * @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, + const char *cmd_prefix, const struct command_registration *cmds, struct target *target) { - return __register_commands(cmd_ctx, parent, cmds, NULL, target); + return __register_commands(cmd_ctx, cmd_prefix, cmds, NULL, target); } /** @@ -292,7 +292,7 @@ static inline int register_commands_override_target(struct command_context *cmd_ * 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 + * @param cmd_prefix 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 @@ -301,10 +301,10 @@ static inline int register_commands_override_target(struct command_context *cmd_ * @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, + const char *cmd_prefix, const struct command_registration *cmds, void *data) { - return __register_commands(cmd_ctx, parent, cmds, data, NULL); + return __register_commands(cmd_ctx, cmd_prefix, cmds, data, NULL); } /** @@ -316,9 +316,6 @@ static inline int register_commands_with_data(struct command_context *cmd_ctx, int unregister_all_commands(struct command_context *cmd_ctx, struct command *parent); -struct command *command_find_in_context(struct command_context *cmd_ctx, - const char *name); - void command_set_output_handler(struct command_context *context, command_output_handler_t output_handler, void *priv); diff --git a/src/pld/pld.c b/src/pld/pld.c index ef7993c5d..9e8c07d80 100644 --- a/src/pld/pld.c +++ b/src/pld/pld.c @@ -187,8 +187,7 @@ static int pld_init(struct command_context *cmd_ctx) if (!pld_devices) return ERROR_OK; - struct command *parent = command_find_in_context(cmd_ctx, "pld"); - return register_commands(cmd_ctx, parent, pld_exec_command_handlers); + return register_commands(cmd_ctx, "pld", pld_exec_command_handlers); } COMMAND_HANDLER(handle_pld_init_command) diff --git a/src/target/etm.c b/src/target/etm.c index 19f3691bd..6dc2bd48c 100644 --- a/src/target/etm.c +++ b/src/target/etm.c @@ -2107,6 +2107,5 @@ static const struct command_registration etm_exec_command_handlers[] = { static int etm_register_user_commands(struct command_context *cmd_ctx) { - struct command *etm_cmd = command_find_in_context(cmd_ctx, "etm"); - return register_commands(cmd_ctx, etm_cmd, etm_exec_command_handlers); + return register_commands(cmd_ctx, "etm", etm_exec_command_handlers); } commit 428938993742f4f961cdc948593d9553f721c321 Author: Antonio Borneo <bor...@gm...> 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, dap and swo/tpiu 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, dap and swo/tpiu). 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 <bor...@gm...> Reviewed-on: http://openocd.zylin.com/5667 Tested-by: jenkins Reviewed-by: Oleksij Rempel <li...@re...> diff --git a/src/helper/command.c b/src/helper/command.c index ecca75db6..89e217382 100644 --- a/src/helper/command.c +++ b/src/helper/command.c @@ -391,8 +391,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; @@ -406,10 +407,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; } @@ -461,13 +464,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) @@ -1057,12 +1053,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 cb088d743..871c064d3 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; }; @@ -242,6 +240,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 @@ -257,8 +259,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 specified context. @@ -272,16 +319,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 hierarchy 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 689e9df9f..ee9d8aafd 100644 --- a/src/target/arm_cti.c +++ b/src/target/arm_cti.c @@ -507,17 +507,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 56442f183..a9277e798 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/arm_tpiu_swo.c b/src/target/arm_tpiu_swo.c index 543b4f008..186ce5d0e 100644 --- a/src/target/arm_tpiu_swo.c +++ b/src/target/arm_tpiu_swo.c @@ -886,14 +886,10 @@ static int arm_tpiu_swo_create(Jim_Interp *interp, struct arm_tpiu_swo_object *o }, COMMAND_REGISTRATION_DONE }; - e = register_commands(cmd_ctx, NULL, obj_commands); + e = register_commands_with_data(cmd_ctx, NULL, obj_commands, obj); if (ERROR_OK != e) return JIM_ERR; - struct command *c = command_find_in_context(cmd_ctx, obj->name); - assert(c); - command_set_handler_data(c, obj); - list_add_tail(&obj->lh, &all_tpiu_swo); return JIM_OK; diff --git a/src/target/target.c b/src/target/target.c index 619a8b490..fa033d351 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -5871,7 +5871,7 @@ static int target_create(Jim_GetOptInfo *goi) }, COMMAND_REGISTRATION_DONE }; - e = register_commands(cmd_ctx, NULL, target_commands); + e = register_commands_override_target(cmd_ctx, NULL, target_commands, target); if (e != ERROR_OK) { if (target->type->deinit_target) target->type->deinit_target(target); @@ -5884,10 +5884,6 @@ static int target_create(Jim_GetOptInfo *goi) return JIM_ERR; } - struct command *c = command_find_in_context(cmd_ctx, cp); - assert(c); - command_set_handler_data(c, target); - /* append to end of list */ append_to_list_all_targets(target); commit 7cd679a2de697065bbd36cde9042516ccf20f0f1 Author: Antonio Borneo <bor...@gm...> Date: Tue May 12 02:36:56 2020 +0200 helper/command: get current target from dedicated API Now that target override is uniformly implemented for all types of commands, there is no need for target-prefixed "native" commands (.jim_handler) to sneakily extract the overridden target from the struct command. Modify the commands to use the standard API get_current_target(). Change-Id: I732a09c3261e56524edd5217634fa409eb97a8c6 Signed-off-by: Antonio Borneo <bor...@gm...> Reviewed-on: http://openocd.zylin.com/5666 Tested-by: jenkins Reviewed-by: Oleksij Rempel <li...@re...> diff --git a/src/target/nds32_cmd.c b/src/target/nds32_cmd.c index 89da845d3..246dbd034 100644 --- a/src/target/nds32_cmd.c +++ b/src/target/nds32_cmd.c @@ -722,8 +722,9 @@ static int jim_nds32_bulk_write(Jim_Interp *interp, int argc, Jim_Obj * const *a return JIM_ERR; } - struct command *c = jim_to_command(goi.interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); int result; result = target_write_buffer(target, address, count * 4, (const uint8_t *)data); @@ -752,8 +753,9 @@ static int jim_nds32_multi_write(Jim_Interp *interp, int argc, Jim_Obj * const * if (e != JIM_OK) return e; - struct command *c = jim_to_command(goi.interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); struct aice_port_s *aice = target_to_aice(target); int result; uint32_t address; @@ -814,8 +816,9 @@ static int jim_nds32_bulk_read(Jim_Interp *interp, int argc, Jim_Obj * const *ar if (goi.argc != 0) return JIM_ERR; - struct command *c = jim_to_command(goi.interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); uint32_t *data = malloc(count * sizeof(uint32_t)); int result; result = target_read_buffer(target, address, count * 4, (uint8_t *)data); @@ -866,8 +869,9 @@ static int jim_nds32_read_edm_sr(Jim_Interp *interp, int argc, Jim_Obj * const * else return ERROR_FAIL; - struct command *c = jim_to_command(goi.interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); struct aice_port_s *aice = target_to_aice(target); char data_str[11]; @@ -915,8 +919,9 @@ static int jim_nds32_write_edm_sr(Jim_Interp *interp, int argc, Jim_Obj * const else return ERROR_FAIL; - struct command *c = jim_to_command(goi.interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); 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 e481d526c..619a8b490 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -5206,24 +5206,27 @@ static int jim_target_configure(Jim_Interp *interp, int argc, Jim_Obj * const *a "missing: -option ..."); return JIM_ERR; } - struct command *c = jim_to_command(goi.interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); return target_configure(&goi, target); } static int jim_target_mem2array(Jim_Interp *interp, int argc, Jim_Obj *const *argv) { - struct command *c = jim_to_command(interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); return target_mem2array(interp, target, argc - 1, argv + 1); } static int jim_target_array2mem(Jim_Interp *interp, int argc, Jim_Obj *const *argv) { - struct command *c = jim_to_command(interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); return target_array2mem(interp, target, argc - 1, argv + 1); } @@ -5255,8 +5258,9 @@ static int jim_target_examine(Jim_Interp *interp, int argc, Jim_Obj *const *argv allow_defer = true; } - struct command *c = jim_to_command(interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); if (!target->tap->enabled) return jim_target_tap_disabled(interp); @@ -5274,8 +5278,9 @@ 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 command *c = jim_to_command(interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); Jim_SetResultBool(interp, target_was_examined(target)); return JIM_OK; @@ -5283,8 +5288,9 @@ 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 command *c = jim_to_command(interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); Jim_SetResultBool(interp, target->defer_examine); return JIM_OK; @@ -5296,8 +5302,9 @@ 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 command *c = jim_to_command(interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); if (target_call_event_callbacks(target, TARGET_EVENT_GDB_HALT) != ERROR_OK) return JIM_ERR; @@ -5311,8 +5318,9 @@ 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 command *c = jim_to_command(interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); if (!target->tap->enabled) return jim_target_tap_disabled(interp); @@ -5349,8 +5357,9 @@ static int jim_target_reset(Jim_Interp *interp, int argc, Jim_Obj *const *argv) if (e != JIM_OK) return e; - struct command *c = jim_to_command(goi.interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); if (!target->tap->enabled) return jim_target_tap_disabled(interp); @@ -5383,8 +5392,9 @@ 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 command *c = jim_to_command(interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); if (!target->tap->enabled) return jim_target_tap_disabled(interp); int e = target->type->halt(target); @@ -5414,8 +5424,9 @@ 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 command *c = jim_to_command(interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); if (!target->tap->enabled) return jim_target_tap_disabled(interp); @@ -5459,8 +5470,9 @@ 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 command *c = jim_to_command(interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); Jim_SetResultString(interp, target_state_name(target), -1); return JIM_OK; } @@ -5479,8 +5491,9 @@ 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 command *c = jim_to_command(interp); - struct target *target = c->jim_handler_data; + struct command_context *cmd_ctx = current_command_context(interp); + assert(cmd_ctx); + struct target *target = get_current_target(cmd_ctx); target_handle_event(target, n->value); return JIM_OK; } commit 462323f123ede2c8da59cb4acb772dc4f98dee79 Author: Antonio Borneo <bor...@gm...> Date: Tue May 12 01:59:06 2020 +0200 helper/command: use one single handler for all the commands Today openocd registers the commands to jim with three methods: 1) "native" commands (.jim_handler) at root level are registered directly as jim commands; 2) "simple" commands (.handler) at root level are registered through the handler script_command(); 3) all other commands not at root level are registered through the handler command_unknown(). Apart from using different handler, other inconsistencies are present: a) command in 1) are not checked for their "mode", so are run with no check about current mode (COMMAND_CONFIG or COMMAND_EXEC); b) target_call_timer_callbacks_now() is called only for "simple" commands and not for "native" commands; c) target override is performed only for "simple" commands and not for "native" commands. Drop script_command() and extend command_unknown() to uniformly handle all the cases above, fixing all the inconsistencies already mentioned. The handler's name command_unknown() is probably not anymore appropriate, but will be renamed in a separate change. Note: today all the commands in a) have mode CONFIG_ANY, apart for "mem2array" and "array2mem" that have mode COMMAND_EXEC. But the latter commands are registered during target init, so do not exist during COMMAND_CONFIG and no issue is present. Change-Id: I67bd6e47eb2c575107251b9192c676c27d4aabae Signed-off-by: Antonio Borneo <bor...@gm...> Reviewed-on: http://openocd.zylin.com/5665 Tested-by: jenkins Reviewed-by: Oleksij Rempel <li...@re...> diff --git a/src/helper/command.c b/src/helper/command.c index b44e4667f..ecca75db6 100644 --- a/src/helper/command.c +++ b/src/helper/command.c @@ -44,9 +44,6 @@ /* nice short description of source file */ #define __THIS__FILE__ "command.c" -static int run_command(struct command_context *context, - struct command *c, const char *words[], unsigned num_words); - struct log_capture_state { Jim_Interp *interp; Jim_Obj *output; @@ -226,34 +223,6 @@ struct command_context *current_command_context(Jim_Interp *interp) return cmd_ctx; } -static int script_command_run(Jim_Interp *interp, - int argc, Jim_Obj * const *argv, struct command *c) -{ - target_call_timer_callbacks_now(); - LOG_USER_N("%s", ""); /* Keep GDB connection alive*/ - - unsigned nwords; - char **words = script_command_args_alloc(argc, argv, &nwords); - if (NULL == words) - return JIM_ERR; - - struct command_context *cmd_ctx = current_command_context(interp); - int retval = run_command(cmd_ctx, c, (const char **)words, nwords); - - script_command_args_free(words, nwords); - return command_retval_set(interp, retval); -} - -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 = jim_to_command(interp); - assert(c); - script_debug(interp, argc, argv); - return script_command_run(interp, argc, argv, c); -} - static struct command *command_root(struct command *c) { while (NULL != c->parent) @@ -387,10 +356,7 @@ static int register_command_handler(struct command_context *cmd_ctx, LOG_DEBUG("registering '%s'...", c->name); #endif - Jim_CmdProc *func = c->handler ? &script_command : &command_unknown; - int retval = Jim_CreateCommand(interp, c->name, func, c, NULL); - - return retval; + return Jim_CreateCommand(interp, c->name, command_unknown, c, NULL); } static struct command *register_command(struct command_context *context, @@ -415,16 +381,12 @@ static struct command *register_command(struct command_context *context, if (NULL == c) return NULL; - int retval = JIM_OK; - if (NULL != cr->jim_handler && NULL == parent) { - retval = Jim_CreateCommand(context->interp, cr->name, - cr->jim_handler, c, NULL); - } else if (NULL != cr->handler || NULL != parent) - retval = register_command_handler(context, command_root(c)); - - if (retval != JIM_OK) { - unregister_command(context, parent, name); - c = NULL; + if (cr->jim_handler || cr->handler) { + int retval = register_command_handler(context, command_root(c)); + if (retval != JIM_OK) { + unregister_command(context, parent, name); + return NULL; + } } return c; } @@ -614,11 +576,8 @@ static bool command_can_run(struct command_context *cmd_ctx, struct command *c) } static int run_command(struct command_context *context, - struct command *c, const char *words[], unsigned num_words) + struct command *c, const char **words, unsigned num_words) { - if (!command_can_run(context, c)) - return ERROR_FAIL; - struct command_invocation cmd = { .ctx = context, .current = c, @@ -626,26 +585,11 @@ 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; cmd.output = Jim_NewEmptyStringObj(context->interp); Jim_IncrRefCount(cmd.output); 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, ' '); @@ -1053,6 +997,23 @@ static int run_usage(Jim_Interp *interp, int argc_valid, int argc, Jim_Obj * con return retval; } +static int exec_command(Jim_Interp *interp, struct command_context *cmd_ctx, + struct command *c, int argc, Jim_Obj *const *argv) +{ + if (c->jim_handler) + return c->jim_handler(interp, argc, argv); + + /* use c->handler */ + unsigned int nwords; + char **words = script_command_args_alloc(argc, argv, &nwords); + if (!words) + return JIM_ERR; + + int retval = run_command(cmd_ctx, c, (const char **)words, nwords); + script_command_args_free(words, nwords); + return command_retval_set(interp, retval); +} + static int command_unknown(Jim_Interp *interp, int argc, Jim_Obj *const *argv) { script_debug(interp, argc, argv); @@ -1079,15 +1040,32 @@ static int command_unknown(Jim_Interp *interp, int argc, Jim_Obj *const *argv) run_usage(interp, count, argc, start); return JIM_ERR; } - /* pass the command through to the intended handler */ - if (c->jim_handler) { - if (!command_can_run(cmd_ctx, c)) - return JIM_ERR; - return (*c->jim_handler)(interp, count, start); - } + if (!command_can_run(cmd_ctx, c)) + return JIM_ERR; + + target_call_timer_callbacks_now(); - return script_command_run(interp, count, start, c); + /* + * 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 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; + + int retval = exec_command(interp, cmd_ctx, c, count, start); + + if (c->jim_handler_data) + cmd_ctx->current_target_override = saved_target_override; + + return retval; } static int jim_command_mode(Jim_Interp *interp, int argc, Jim_Obj *const *argv) ----------------------------------------------------------------------- Summary of changes: src/flash/nand/tcl.c | 4 +- src/flash/nor/esirisc_flash.c | 4 +- src/flash/nor/tcl.c | 3 +- src/helper/command.c | 154 ++++++++++++++++++------------------------ src/helper/command.h | 76 +++++++++++++++------ src/pld/pld.c | 3 +- src/target/arm_cti.c | 8 +-- src/target/arm_dap.c | 8 +-- src/target/arm_tpiu_swo.c | 6 +- src/target/etm.c | 3 +- src/target/nds32_cmd.c | 25 ++++--- src/target/target.c | 71 ++++++++++--------- 12 files changed, 188 insertions(+), 177 deletions(-) hooks/post-receive -- Main OpenOCD repository |