From 93f16eed4d4db379671293277e9b6e7d5aba6c9b Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Fri, 15 Aug 2025 14:53:34 +0200 Subject: [PATCH] command: fix OpenOCD commands return value for next jimtcl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JimTcl has been so far quite comfortable with new commands that return error codes not supported by JimTcl itself. This has been exploited by OpenOCD, allowing the OpenOCD commands to return OpenOCD error codes mixed with JimTcl error code. With the change [1] merged in JimTcl branch 'master' for 0.84, any negative value returned by a command gets interpreted as a syntax error detected at runtime by the command itself; JimTcl dumps the correct syntax and returns a valid JimTcl error code that replaces the negative value. Since all OpenOCD error codes are negative values, they are all taken as syntax errors by the new JimTcl. E.g.: openocd -c exit dumps wrong # args: should be "exit ..." Actually OpenOCD does not need the OpenOCD error code from the commands, with the exception of the codes: [a] ERROR_COMMAND_SYNTAX_ERROR, used internally by the command dispatcher, before returning to JimTcl; [b] ERROR_COMMAND_CLOSE_CONNECTION, to alert the telnet server that the current connection should be closed. With [a] already used internally, only [b] needs to be propagated through JimTcl and back to the OpenOCD caller. Map the OpenOCD error code ERROR_COMMAND_CLOSE_CONNECTION to the existing JimTcl error code JIM_EXIT, originally used only by JimTcl 'exit' command. Detect JIM_EXIT in command_run_line() and return to the caller the original ERROR_COMMAND_CLOSE_CONNECTION. Let exec_command(), and also its caller jim_command_dispatch(), to only return JimTcl error codes. Rename it to report the change. Modify the test suite as now a syntax error does not returns -601 anymore. While there, drop the association key "retval" as it's not used. Note: after this change there is no real need to replace the JimTcl command 'exit' with the OpenOCD version as both produce the same result. But I prefer keeping the code as is to mask any future change in the related JimTcl code. Link: https://github.com/msteveb/jimtcl/commit/5669e84aad22 [1] Change-Id: Ibd7aaeccdf4d7c9efe72aa71909aef83be5ecd27 Signed-off-by: Antonio Borneo Reported-by: Andrzej Sierżęga Reviewed-on: https://review.openocd.org/c/openocd/+/9084 Tested-by: jenkins --- src/helper/command.c | 56 ++++++++++++++-------------------- testing/tcl_commands/utils.tcl | 2 +- 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/src/helper/command.c b/src/helper/command.c index d6993ab76..742eea1b1 100644 --- a/src/helper/command.c +++ b/src/helper/command.c @@ -417,7 +417,7 @@ static bool command_can_run(struct command_context *cmd_ctx, struct command *c, return false; } -static int exec_command(Jim_Interp *interp, struct command_context *context, +static int jim_exec_command(Jim_Interp *interp, struct command_context *context, struct command *c, int argc, Jim_Obj * const *argv) { /* use c->handler */ @@ -467,14 +467,14 @@ static int exec_command(Jim_Interp *interp, struct command_context *context, free(words); - int *return_retval = Jim_GetAssocData(interp, "retval"); - if (return_retval) - *return_retval = retval; - if (retval == ERROR_OK) return JIM_OK; - return retval; + // used by telnet server to close one connection + if (retval == ERROR_COMMAND_CLOSE_CONNECTION) + return JIM_EXIT; + + return JIM_ERR; } int command_run_line(struct command_context *context, char *line) @@ -484,7 +484,6 @@ int command_run_line(struct command_context *context, char *line) * results */ /* run the line thru a script engine */ - int retval = ERROR_FAIL; int retcode; /* Beware! This code needs to be reentrant. It is also possible * for OpenOCD commands to be invoked directly from Tcl. This would @@ -499,20 +498,17 @@ int command_run_line(struct command_context *context, char *line) Jim_DeleteAssocData(interp, "context"); retcode = Jim_SetAssocData(interp, "context", NULL, context); if (retcode == JIM_OK) { - /* associated the return value */ - Jim_DeleteAssocData(interp, "retval"); - retcode = Jim_SetAssocData(interp, "retval", NULL, &retval); - if (retcode == JIM_OK) { - retcode = Jim_Eval_Named(interp, line, NULL, 0); - - Jim_DeleteAssocData(interp, "retval"); - } + retcode = Jim_Eval_Named(interp, line, NULL, 0); Jim_DeleteAssocData(interp, "context"); int inner_retcode = Jim_SetAssocData(interp, "context", NULL, old_context); if (retcode == JIM_OK) retcode = inner_retcode; } context->current_target_override = saved_target_override; + + if (retcode == JIM_RETURN) + retcode = interp->returnCode; + if (retcode == JIM_OK) { const char *result; int reslen; @@ -522,25 +518,19 @@ int command_run_line(struct command_context *context, char *line) command_output_text(context, result); command_output_text(context, "\n"); } - retval = ERROR_OK; - } else if (retcode == JIM_EXIT) { - /* ignore. - * exit(Jim_GetExitCode(interp)); */ - } else if (retcode == ERROR_COMMAND_CLOSE_CONNECTION) { - return retcode; - } else { - Jim_MakeErrorMessage(interp); - /* error is broadcast */ - LOG_USER("%s", Jim_GetString(Jim_GetResult(interp), NULL)); - - if (retval == ERROR_OK) { - /* It wasn't a low level OpenOCD command that failed */ - return ERROR_FAIL; - } - return retval; + return ERROR_OK; } - return retval; + if (retcode == JIM_EXIT) { + // used by telnet server to close one connection + return ERROR_COMMAND_CLOSE_CONNECTION; + } + + Jim_MakeErrorMessage(interp); + /* error is broadcast */ + LOG_USER("%s", Jim_GetString(Jim_GetResult(interp), NULL)); + + return ERROR_FAIL; } int command_run_linef(struct command_context *context, const char *format, ...) @@ -867,7 +857,7 @@ static int jim_command_dispatch(Jim_Interp *interp, int argc, Jim_Obj * const *a if (c->jim_override_target) cmd_ctx->current_target_override = c->jim_override_target; - int retval = exec_command(interp, cmd_ctx, c, argc, argv); + int retval = jim_exec_command(interp, cmd_ctx, c, argc, argv); if (c->jim_override_target) cmd_ctx->current_target_override = saved_target_override; diff --git a/testing/tcl_commands/utils.tcl b/testing/tcl_commands/utils.tcl index dfc896661..65e52d2fe 100644 --- a/testing/tcl_commands/utils.tcl +++ b/testing/tcl_commands/utils.tcl @@ -32,7 +32,7 @@ namespace eval testing_helpers { } proc check_syntax_err script { - tailcall check_for_error -601 {} $script + tailcall check_for_error 1 {} $script } proc check_matches {pattern script} {