Re: [PATCH v3 09/49] semihosting: Adjust error checking in common_semi_cb

[Peter Maydell]

On Sat, 21 May 2022 at 01:04, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The err parameter is non-zero if and only if an error occured.
> Use this instead of ret == -1 for determining if we need to
> update the saved errno.

The gdb protocol isn't 100% clear on this, but what it says is:
https://sourceware.org/gdb/onlinedocs/gdb/The-F-Reply-Packet.html#The-F-Reply-Packet

# retcode is the return code of the system call as hexadecimal value.
# errno is the errno set by the call, in protocol-specific representation.
# This parameter can be omitted if the call was successful.

(and from the implementation in gdb it is basically just returning
the return value from a syscall plus errno).

That implies that our current code is right, in that the
way to check for "did the call fail" is to look at the
retcode, not the errno (in the same way that if you make a
native syscall or library call you look first at its return
value, not at errno). There's nothing in the protocol text
that makes a guarantee that the errno value is non-0 if and
only if the call failed.

(I think gdb's implementation happens to only set errno to
non-0 on failures, and lldb doesn't implement the file-io
extension at all as far as I can see, so there might be scope
for getting the protocol definition tightened up I guess.)

> This fixes the errno setting of SYS_ISTTY, which returns 0 on
> error, not -1.

The gdb implementation of the isatty call returns 0 or 1 on
success, and -1 on failure (though the only failure mode it has
is "you messed up the protocol packet format"):
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/remote-fileio.c;h=fe191fb6069a53a3844656a81e77069afa781946;hb=HEAD#l1039

thanks
-- PMM