command: fix OpenOCD commands return value for next jimtcl

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 <borneo.antonio@gmail.com>
Reported-by: Andrzej Sierżęga <asier70@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/9084
Tested-by: jenkins

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;

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} {