Re: [PATCH v4 27/53] semihosting: Split out semihost_sys_open

[Luc Michel]

On 13:45 Tue 07 Jun     , Richard Henderson wrote:
> Split out the non-ARM specific portions of SYS_OPEN to a
> reusable function.  This handles gdb and host file i/o.
> 
> Add helpers to validate the length of the filename string.
> Prepare for usage by other semihosting by allowing the
> filename length parameter to be 0, and calling strlen.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/semihosting/syscalls.h |  25 ++++++
>  semihosting/arm-compat-semi.c  |  52 ++---------
>  semihosting/guestfd.c          |   5 ++
>  semihosting/syscalls.c         | 156 +++++++++++++++++++++++++++++++++
>  semihosting/meson.build        |   1 +
>  5 files changed, 193 insertions(+), 46 deletions(-)
>  create mode 100644 include/semihosting/syscalls.h
>  create mode 100644 semihosting/syscalls.c
> 

[...]

>          } else {
> -            hostfd = open(s, open_modeflags[arg1], 0644);
> -            if (hostfd < 0) {
> -                ret = -1;
> -                err = errno;
> -            } else {
> -                ret = alloc_guestfd();
> -                associate_guestfd(ret, hostfd);
> -            }
> +            semihost_sys_open(cs, common_semi_cb, arg0, arg2 + 1,
> +                              gdb_open_modeflags[arg1], 0644);

You're missing a unlock_user(s, arg0, 0); here