refactor command registration

Refactors the command registration to use helpers to simplify the code.
The unregistration routines were made more flexible by allowing them
to operate on a single command, such that one can remove all of a
commands children in one step (perhaps before adding back a 'config'
subcommand that allows getting the others back).  Eliminates a bit
of duplicated code and adds full API documentation for these routines.

diff --git a/src/helper/command.c b/src/helper/command.c
index 538c07bb917392981622af66327d30c6774fe649..87a898f293eb9b868a3ea1287e7ce82c788f60c5 100644 (file)
--- a/src/helper/command.c
+++ b/src/helper/command.c
@@ -233,33 +233,69 @@ static void command_add_child(struct command **head, struct command *c)
        cc->next = c;
 }
 
-struct command* register_command(struct command_context *context,
-               struct command *parent, char *name, command_handler_t handler,
-               enum command_mode mode, char *help)
+static struct command **command_list_for_parent(
+               struct command_context *cmd_ctx, struct command *parent)
 {
-       if (!context || !name)
-               return NULL;
+       return parent ? &parent->children : &cmd_ctx->commands;
+}
 
-       struct command **head = parent ? &parent->children : &context->commands;
-       struct command *c = command_find(*head, name);
-       if (NULL != c)
-               return c;
+static struct command *command_new(struct command_context *cmd_ctx,
+               struct command *parent, const char *name,
+               command_handler_t handler, enum command_mode mode,
+               const char *help)
+{
+       assert(name);
 
-       c = malloc(sizeof(struct command));
+       struct command *c = malloc(sizeof(struct command));
+       memset(c, 0, sizeof(struct command));
 
        c->name = strdup(name);
        c->parent = parent;
-       c->children = NULL;
        c->handler = handler;
        c->mode = mode;
-       c->next = NULL;
 
-       command_add_child(head, c);
+       command_add_child(command_list_for_parent(cmd_ctx, parent), c);
 
        command_helptext_add(command_name_list(c), help);
 
-       /* just a placeholder, no handler */
-       if (c->handler == NULL)
+       return c;
+}
+static void command_free(struct command *c)
+{
+       /// @todo if command has a handler, unregister its jim command!
+
+       while (NULL != c->children)
+       {
+               struct command *tmp = c->children;
+               c->children = tmp->next;
+               command_free(tmp);
+       }
+
+       if (c->name)
+               free(c->name);
+       free(c);
+}
+
+struct command* register_command(struct command_context *context,
+               struct command *parent, const char *name,
+               command_handler_t handler, enum command_mode mode,
+               const char *help)
+{
+       if (!context || !name)
+               return NULL;
+
+       struct command **head = command_list_for_parent(context, parent);
+       struct command *c = command_find(*head, name);
+       if (NULL != c)
+       {
+               LOG_ERROR("command '%s' is already registered in '%s' context",
+                               name, parent ? parent->name : "<global>");
+               return c;
+       }
+
+       c = command_new(context, parent, name, handler, mode, help);
+       /* if allocation failed or it is a placeholder (no handler), we're done */
+       if (NULL == c || NULL == c->handler)
                return c;
 
        const char *full_name = command_name(c, '_');
@@ -281,85 +317,43 @@ struct command* register_command(struct command_context *context,
        return c;
 }
 
-int unregister_all_commands(struct command_context *context)
+int unregister_all_commands(struct command_context *context,
+               struct command *parent)
 {
-       struct command *c, *c2;
-
        if (context == NULL)
                return ERROR_OK;
 
-       while (NULL != context->commands)
+       struct command **head = command_list_for_parent(context, parent);
+       while (NULL != *head)
        {
-               c = context->commands;
-
-               while (NULL != c->children)
-               {
-                       c2 = c->children;
-                       c->children = c->children->next;
-                       free(c2->name);
-                       c2->name = NULL;
-                       free(c2);
-                       c2 = NULL;
-               }
-
-               context->commands = context->commands->next;
-
-               free(c->name);
-               c->name = NULL;
-               free(c);
-               c = NULL;
+               struct command *tmp = *head;
+               *head = tmp->next;
+               command_free(tmp);
        }
 
        return ERROR_OK;
 }
 
-int unregister_command(struct command_context *context, char *name)
+int unregister_command(struct command_context *context,
+               struct command *parent, const char *name)
 {
-       struct command *c, *p = NULL, *c2;
-
        if ((!context) || (!name))
                return ERROR_INVALID_ARGUMENTS;
 
-       /* find command */
-       c = context->commands;
-
-       while (NULL != c)
+       struct command *p = NULL;
+       struct command **head = command_list_for_parent(context, parent);
+       for (struct command *c = *head; NULL != c; p = c, c = c->next)
        {
-               if (strcmp(name, c->name) == 0)
-               {
-                       /* unlink command */
-                       if (p)
-                       {
-                               p->next = c->next;
-                       }
-                       else
-                       {
-                               /* first element in command list */
-                               context->commands = c->next;
-                       }
+               if (strcmp(name, c->name) != 0)
+                       continue;
 
-                       /* unregister children */
-                       while (NULL != c->children)
-                       {
-                               c2 = c->children;
-                               c->children = c->children->next;
-                               free(c2->name);
-                               c2->name = NULL;
-                               free(c2);
-                               c2 = NULL;
-                       }
-
-                       /* delete command */
-                       free(c->name);
-                       c->name = NULL;
-                       free(c);
-                       c = NULL;
-                       return ERROR_OK;
-               }
+               if (p)
+                       p->next = c->next;
+               else
+                       *head = c->next;
 
-               /* remember the last command for unlinking */
-               p = c;
-               c = c->next;
+               command_free(c);
+               return ERROR_OK;
        }
 
        return ERROR_OK;

diff --git a/src/helper/command.h b/src/helper/command.h
index def0935a737e7a734d22fe89015da8938b8a8ec8..a7b422adf899594a818dd8d05ac27634b40158e3 100644 (file)
--- a/src/helper/command.h
+++ b/src/helper/command.h
@@ -176,12 +176,53 @@ struct command
  */
 char *command_name(struct command *c, char delim);
 
-struct command* register_command(struct command_context *context,
-               struct command *parent, char *name, command_handler_t handler,
-               enum command_mode mode, char *help);
+/**
+ * Register a command @c handler that can be called from scripts during
+ * the execution @c mode specified.
+ *
+ * If @c parent is non-NULL, the new command will be registered as a
+ * sub-command under it; otherwise, it will be available as a top-level
+ * command.
+ *
+ * A conventioal format should be used for help strings, to provide both
+ * usage and basic information:
+ * @code
+ * "@<options@> ... - some explanation text"
+ * @endcode
+ *
+ * @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 name The name of the command to register, which must not have
+ * been registered previously.
+ * @param handler The callback function that will be called.  If NULL,
+ * then the command serves as a placeholder for its children or a script.
+ * @param mode The command mode(s) in which this command may be run.
+ * @param help The help text that will be displayed to the user.
+ * @returns The new command, if successful; otherwise, NULL.
+ */
+struct command* register_command(struct command_context *cmd_ctx,
+               struct command *parent, const char *name,
+               command_handler_t handler, enum command_mode mode,
+               const char *help);
 
-int unregister_command(struct command_context *context, char *name);
-int unregister_all_commands(struct command_context *context);
+/**
+ * Unregisters command @c name from the given context, @c cmd_ctx.
+ * @param cmd_ctx The context of the registered command.
+ * @param parent The parent of the given command, or NULL.
+ * @param name The name of the command to unregister.
+ * @returns ERROR_OK on success, or an error code.
+ */
+int unregister_command(struct command_context *cmd_ctx,
+               struct command *parent, const char *name);
+/**
+ * Unregisters all commands from the specfied context.
+ * @param cmd_ctx The context that will be cleared of registered commands.
+ * @param parent If given, only clear commands from under this one command.
+ * @returns ERROR_OK on success, or an error code.
+ */
+int unregister_all_commands(struct command_context *cmd_ctx,
+               struct command *parent);
 
 void command_set_output_handler(struct command_context* context,
                command_output_handler_t output_handler, void *priv);

diff --git a/src/openocd.c b/src/openocd.c
index b7781a6b6d37332693838815f662347bdcfd80ca..8e8ceac45eb8ece54df1b877b6438d1ae2e1631f 100644 (file)
--- a/src/openocd.c
+++ b/src/openocd.c
@@ -278,7 +278,7 @@ int openocd_main(int argc, char *argv[])
        httpd_stop();
 #endif
 
-       unregister_all_commands(cmd_ctx);
+       unregister_all_commands(cmd_ctx, NULL);
 
        /* free commandline interface */
        command_done(cmd_ctx);