From 7f260f5009a774f2d66b5f3037f8f595c6881d4d Mon Sep 17 00:00:00 2001 From: Paul Fertser Date: Thu, 4 Apr 2019 09:54:09 +0200 Subject: [PATCH] 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 Signed-off-by: Tomas Vanek Signed-off-by: Antonio Borneo Reviewed-on: http://openocd.zylin.com/1815 Tested-by: jenkins --- src/helper/command.c | 15 +++++++++++---- src/helper/command.h | 1 + 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/helper/command.c b/src/helper/command.c index be131bb3b9..11c9e5f526 100644 --- 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); - 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. */ - 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); @@ -533,7 +533,7 @@ void command_print(struct command_invocation *cmd, const char *format, ...) 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 @@ -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. */ - 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); @@ -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; + cmd.output = Jim_NewEmptyStringObj(context->interp); + Jim_IncrRefCount(cmd.output); + 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); + } else { + /* Use the command output as the Tcl result */ + Jim_SetResult(context->interp, cmd.output); } + Jim_DecrRefCount(context->interp, cmd.output); return retval; } diff --git a/src/helper/command.h b/src/helper/command.h index 41cdc0b9d2..733ba42dd1 100644 --- 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; + Jim_Obj *output; }; /** -- 2.30.2