Re: [PATCH 2/4] ssh.exp: do not use '&&' with 'XYZ${?}ZYX' logic

On 3/24/26 16:20, Joseph Myers wrote:
> There is a bogus comment "We use && here, as otherwise the echo always
> works, which makes it look like execution succeeded when in reality it
> failed.".  But the whole point of the XYZ${?}ZYX logic, taken from
> rsh.exp, is that code a few lines below uses a regular expression to
> extract the actual exit status from the test output, which the use of
> '&&' breaks when the test program does fail.  Furthermore, if the
> intent was to yield nonzero exit status from local_exec when the test
> program fails, the subsequent rm would prevent that from working by
> yielding the exit status from rm instead.
> ---
>   lib/ssh.exp | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/lib/ssh.exp b/lib/ssh.exp
> index bb827f4..b64bf73 100644
> --- a/lib/ssh.exp
> +++ b/lib/ssh.exp
> @@ -168,9 +168,7 @@ proc ssh_exec { boardname program pargs inp outp } {
>   	set inp "/dev/null"
>       }
>   
> -    # We use && here, as otherwise the echo always works, which makes it look
> -    # like execution succeeded when in reality it failed.
> -    set ret [local_exec "$SSH $ssh_useropts $ssh_user$hostname sh -c '$program $pargs 2>&1 && echo XYZ\\\${?}ZYX \\; rm -f $program'" $inp $outp $timeout]
> +    set ret [local_exec "$SSH $ssh_useropts $ssh_user$hostname sh -c '$program $pargs 2>&1 ; echo XYZ\\\${?}ZYX \\; rm -f $program'" $inp $outp $timeout]
>       set status [lindex $ret 0]
>       set output [lindex $ret 1]
>   

This corrects a logic error; applied.

The echo(1) is used to report the exit status of the program, but using 
&& means that the report only appears if the program completes 
successfully, which is ridiculous because an error is exactly when you 
most want the exit code.


-- Jacob