helper/command: Handle Tcl return values consistently

Rationale:

1. There's logging output and there're return values;

2. If a function should return something, it should do it explicitly,
same for logging;

3. Interactive interfaces (telnet, Gdb and Tcl RPC) must always return
the result of the evaluation for the given expression. You can
suppress this output by adding ``; after 0'' to the end of your
expression.

4. Some commands "throw an exception" and if you want to be able to
collect both the return value (when it succeeds) and the log output
(when something goes wrong) you can use do like this:

set log_output [capture "catch {reset_config} return_value"]

So what I'm proposing is following:

1. Every jim_handler command should set the return value the standard
JimTcl way, without any tricks. If it needs to print some logging
output, it should use LOG_* functions.

2. The usual commands (COMMANDS) can easily construct their return
value by appending strings with command_print() and
command_print_sameline(). This required changing "struct
command_invocation" and passing a pointer to it to command_print*
functions.

The code is already functional, please test and comment.

TODO items:

1. Modify all jim_handler commands to properly return or log values
(some of them are commented out for now in this patch).

2. Properly document "capture" command and provide a convenience
function to automate log_output + return_value gathering.

3. Add appropriate Doxygen comments.

4. Add Tcl RPC interface description to the manual, all the example
clients in different languages (from the mailing list) to contrib/.

This change is the core part of http://openocd.zylin.com/1815 from
Paul Fertser. It has been extracted and rebased to simplify the
review and provided again as 1815.

Change-Id: I675c91aa9da1e4e7c6f0a8fe6112a00550b9e4db
Signed-off-by: Paul Fertser <fercerpav@gmail.com>
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: http://openocd.zylin.com/1815
Tested-by: jenkins

diff --git a/src/helper/command.c b/src/helper/command.c
index be131bb3b93840d3b414e11fbd11960da39fd992..11c9e5f5265bb804fbd30fa8cdf798d58cfcc2a3 100644 (file)
--- a/src/helper/command.c
+++ b/src/helper/command.c
@@ -510,13 +510,13 @@ void command_print_sameline(struct command_invocation *cmd, const char *format,
        va_start(ap, format);
 
        string = alloc_vprintf(format, ap);
        va_start(ap, format);
 
        string = alloc_vprintf(format, ap);

-       if (string != NULL) {
+       if (string != NULL && cmd) {

                /* we want this collected in the log + we also want to pick it up as a tcl return
                 * value.
                 *
                 * The latter bit isn't precisely neat, but will do for now.
                 */
                /* we want this collected in the log + we also want to pick it up as a tcl return
                 * value.
                 *
                 * The latter bit isn't precisely neat, but will do for now.
                 */

-               LOG_USER_N("%s", string);
+               Jim_AppendString(cmd->ctx->interp, cmd->output, string, -1);

                /* We already printed it above
                 * command_output_text(context, string); */
                free(string);
                /* We already printed it above
                 * command_output_text(context, string); */
                free(string);


@@ -533,7 +533,7 @@ void command_print(struct command_invocation *cmd, const char *format, ...)
        va_start(ap, format);
 
        string = alloc_vprintf(format, ap);
        va_start(ap, format);
 
        string = alloc_vprintf(format, ap);

-       if (string != NULL) {
+       if (string != NULL && cmd) {

                strcat(string, "\n");   /* alloc_vprintf guaranteed the buffer to be at least one
                                         *char longer */
                /* we want this collected in the log + we also want to pick it up as a tcl return
                strcat(string, "\n");   /* alloc_vprintf guaranteed the buffer to be at least one
                                         *char longer */
                /* we want this collected in the log + we also want to pick it up as a tcl return


@@ -541,7 +541,7 @@ void command_print(struct command_invocation *cmd, const char *format, ...)
                 *
                 * The latter bit isn't precisely neat, but will do for now.
                 */
                 *
                 * The latter bit isn't precisely neat, but will do for now.
                 */

-               LOG_USER_N("%s", string);
+               Jim_AppendString(cmd->ctx->interp, cmd->output, string, -1);

                /* We already printed it above
                 * command_output_text(context, string); */
                free(string);
                /* We already printed it above
                 * command_output_text(context, string); */
                free(string);


@@ -628,6 +628,9 @@ static int run_command(struct command_context *context,
        if (c->jim_handler_data)
                context->current_target_override = c->jim_handler_data;
 
        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)
        int retval = c->handler(&cmd);
 
        if (c->jim_handler_data)


@@ -650,7 +653,11 @@ static int run_command(struct command_context *context,
                LOG_DEBUG("Command '%s' failed with error code %d",
                                        full_name ? full_name : c->name, retval);
                free(full_name);
                LOG_DEBUG("Command '%s' failed with error code %d",
                                        full_name ? full_name : c->name, retval);
                free(full_name);

+       } else {
+               /* Use the command output as the Tcl result */
+               Jim_SetResult(context->interp, cmd.output);

        }
        }

+       Jim_DecrRefCount(context->interp, cmd.output);

 
        return retval;
 }
 
        return retval;
 }

diff --git a/src/helper/command.h b/src/helper/command.h
index 41cdc0b9d2de8a0e26dce0f12b27b87f95119d42..733ba42dd1ef5ae113aaa1cc7c17d52407ca658f 100644 (file)
--- a/src/helper/command.h
+++ b/src/helper/command.h
@@ -79,6 +79,7 @@ struct command_invocation {
        const char *name;
        unsigned argc;
        const char **argv;
        const char *name;
        unsigned argc;
        const char **argv;

+       Jim_Obj *output;

 };
 
 /**
 };
 
 /**