Re: [PATCH v4 36/53] semihosting: Split out semihost_sys_rename

[Luc Michel]

On 13:45 Tue 07 Jun     , Richard Henderson wrote:
> Split out the non-ARM specific portions of SYS_RENAME to a
> reusable function.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Luc Michel <lmichel@kalray.eu>

> ---
>  include/semihosting/syscalls.h |  4 +++
>  semihosting/arm-compat-semi.c  | 21 +------------
>  semihosting/syscalls.c         | 57 ++++++++++++++++++++++++++++++++++
>  3 files changed, 62 insertions(+), 20 deletions(-)
> 
> diff --git a/include/semihosting/syscalls.h b/include/semihosting/syscalls.h
> index 748a4b5e47..21430aa0ef 100644
> --- a/include/semihosting/syscalls.h
> +++ b/include/semihosting/syscalls.h
> @@ -52,4 +52,8 @@ void semihost_sys_flen(CPUState *cs, 
> gdb_syscall_complete_cb fstat_cb,
>  void semihost_sys_remove(CPUState *cs, gdb_syscall_complete_cb complete,
>                           target_ulong fname, target_ulong fname_len);
>  
> +void semihost_sys_rename(CPUState *cs, gdb_syscall_complete_cb complete,
> +                         target_ulong oname, target_ulong oname_len,
> +                         target_ulong nname, target_ulong nname_len);
> +
>  #endif /* SEMIHOSTING_SYSCALLS_H */
> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
> index 6c9d50176a..da5c80b3d5 100644
> --- a/semihosting/arm-compat-semi.c
> +++ b/semihosting/arm-compat-semi.c
> @@ -491,26 +491,7 @@ void do_common_semihosting(CPUState *cs)
>          GET_ARG(1);
>          GET_ARG(2);
>          GET_ARG(3);
> -        if (use_gdb_syscalls()) {
> -            gdb_do_syscall(common_semi_cb, "rename,%s,%s",
> -                           arg0, (int)arg1 + 1, arg2, (int)arg3 + 1);
> -        } else {
> -            char *s2;
> -
> -            s = lock_user_string(arg0);
> -            if (!s) {
> -                goto do_fault;
> -            }
> -            s2 = lock_user_string(arg2);
> -            if (!s2) {
> -                unlock_user(s, arg0, 0);
> -                goto do_fault;
> -            }
> -            ret = rename(s, s2);
> -            unlock_user(s2, arg2, 0);
> -            unlock_user(s, arg0, 0);
> -            common_semi_cb(cs, ret, ret ? errno : 0);
> -        }
> +        semihost_sys_rename(cs, common_semi_cb, arg0, arg1 + 1, arg2, arg3 + 
> 1);
>          break;
>  
>      case TARGET_SYS_CLOCK:
> diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c
> index 5ec4e8f827..223916b110 100644
> --- a/semihosting/syscalls.c
> +++ b/semihosting/syscalls.c
> @@ -145,6 +145,26 @@ static void gdb_remove(CPUState *cs, 
> gdb_syscall_complete_cb complete,
>      gdb_do_syscall(complete, "unlink,%s", fname, len);
>  }
>  
> +static void gdb_rename(CPUState *cs, gdb_syscall_complete_cb complete,
> +                       target_ulong oname, target_ulong oname_len,
> +                       target_ulong nname, target_ulong nname_len)
> +{
> +    int olen, nlen;
> +
> +    olen = validate_strlen(cs, oname, oname_len);
> +    if (olen < 0) {
> +        complete(cs, -1, -olen);
> +        return;
> +    }
> +    nlen = validate_strlen(cs, nname, nname_len);
> +    if (nlen < 0) {
> +        complete(cs, -1, -nlen);
> +        return;
> +    }
> +
> +    gdb_do_syscall(complete, "rename,%s,%s", oname, olen, nname, nlen);
> +}
> +
>  /*
>   * Host semihosting syscall implementations.
>   */
> @@ -307,6 +327,32 @@ static void host_remove(CPUState *cs, 
> gdb_syscall_complete_cb complete,
>      unlock_user(p, fname, 0);
>  }
>  
> +static void host_rename(CPUState *cs, gdb_syscall_complete_cb complete,
> +                        target_ulong oname, target_ulong oname_len,
> +                        target_ulong nname, target_ulong nname_len)
> +{
> +    CPUArchState *env G_GNUC_UNUSED = cs->env_ptr;
> +    char *ostr, *nstr;
> +    int ret;
> +
> +    ret = validate_lock_user_string(&ostr, cs, oname, oname_len);
> +    if (ret < 0) {
> +        complete(cs, -1, -ret);
> +        return;
> +    }
> +    ret = validate_lock_user_string(&nstr, cs, nname, nname_len);
> +    if (ret < 0) {
> +        unlock_user(ostr, oname, 0);
> +        complete(cs, -1, -ret);
> +        return;
> +    }
> +
> +    ret = rename(ostr, nstr);
> +    complete(cs, ret, ret ? errno : 0);
> +    unlock_user(ostr, oname, 0);
> +    unlock_user(nstr, nname, 0);
> +}
> +
>  /*
>   * Static file semihosting syscall implementations.
>   */
> @@ -562,3 +608,14 @@ void semihost_sys_remove(CPUState *cs, 
> gdb_syscall_complete_cb complete,
>          host_remove(cs, complete, fname, fname_len);
>      }
>  }
> +
> +void semihost_sys_rename(CPUState *cs, gdb_syscall_complete_cb complete,
> +                         target_ulong oname, target_ulong oname_len,
> +                         target_ulong nname, target_ulong nname_len)
> +{
> +    if (use_gdb_syscalls()) {
> +        gdb_rename(cs, complete, oname, oname_len, nname, nname_len);
> +    } else {
> +        host_rename(cs, complete, oname, oname_len, nname, nname_len);
> +    }
> +}
> -- 
> 2.34.1
> 
> 
> 
> 
> To declare a filtering error, please use the following link : 
> https://www.security-mail.net/reporter.php?mid=db2a.629fd094.619c9.0&r=lmichel%40kalrayinc.com&s=qemu-devel-bounces%2Blmichel%3Dkalrayinc.com%40nongnu.org&o=%5BPATCH+v4+36%2F53%5D+semihosting%3A+Split+out+semihost_sys_rename&verdict=C&c=a6b432f19d21e185c7e45b7892ef238d104b49ae
> 

--