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 <borneo.antonio@gmail.com>
Reviewed-on: http://openocd.zylin.com/5667
Tested-by: jenkins
Reviewed-by: Oleksij Rempel <linux@rempel-privat.de>

diff --git a/src/helper/command.c b/src/helper/command.c
index ecca75db6f320e30dfaf710fc1108949bb1b5a26..89e217382bc581d309fbba70083ef560b34786aa 100644 (file)
--- 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 cb088d74388aefa2194bc23ca9dffacdf5fde4ef..871c064d3b31fe1ca606a21d116119824533d864 100644 (file)
--- 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 689e9df9fe75bb8bc1a3a0fca099102bf6dac443..ee9d8aafd40e920682d2a1e5384067ef4533d57f 100644 (file)
--- 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 56442f1835269f589caad87009c6368cdef1dbf5..a9277e79871a76975cbc95d88cc4fe7b17e89b04 100644 (file)
--- 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 543b4f00849c8aefb2e374a7c4854b4a8f16a772..186ce5d0e50179a680197a62e2c3d20456e540c4 100644 (file)
--- 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 619a8b49088b38cd6de988c83d9b2979865e38b0..fa033d35158b82f2d7b85d2b07cf671ad2b9e151 100644 (file)
--- 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);