Re: [PATCH v4 37/53] semihosting: Split out semihost_sys_system

2022-06-24 Thread Luc Michel
On 13:45 Tue 07 Jun , Richard Henderson wrote:
> Split out the non-ARM specific portions of SYS_SYSTEM to a
> reusable function.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Luc Michel 

> ---
>  include/semihosting/syscalls.h |  3 +++
>  semihosting/arm-compat-semi.c  | 12 +-
>  semihosting/syscalls.c | 40 ++
>  3 files changed, 44 insertions(+), 11 deletions(-)
> 
> diff --git a/include/semihosting/syscalls.h b/include/semihosting/syscalls.h
> index 21430aa0ef..c9f9e66be1 100644
> --- a/include/semihosting/syscalls.h
> +++ b/include/semihosting/syscalls.h
> @@ -56,4 +56,7 @@ 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);
>  
> +void semihost_sys_system(CPUState *cs, gdb_syscall_complete_cb complete,
> + target_ulong cmd, target_ulong cmd_len);
> +
>  #endif /* SEMIHOSTING_SYSCALLS_H */
> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
> index da5c80b3d5..8a4e22a20a 100644
> --- a/semihosting/arm-compat-semi.c
> +++ b/semihosting/arm-compat-semi.c
> @@ -506,17 +506,7 @@ void do_common_semihosting(CPUState *cs)
>  case TARGET_SYS_SYSTEM:
>  GET_ARG(0);
>  GET_ARG(1);
> -if (use_gdb_syscalls()) {
> -gdb_do_syscall(common_semi_cb, "system,%s", arg0, (int)arg1 + 1);
> -break;
> -}
> -s = lock_user_string(arg0);
> -if (!s) {
> -goto do_fault;
> -}
> -ret = system(s);
> -unlock_user(s, arg0, 0);
> -common_semi_cb(cs, ret, ret == -1 ? errno : 0);
> +semihost_sys_system(cs, common_semi_cb, arg0, arg1 + 1);
>  break;
>  
>  case TARGET_SYS_ERRNO:
> diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c
> index 223916b110..de846ced32 100644
> --- a/semihosting/syscalls.c
> +++ b/semihosting/syscalls.c
> @@ -165,6 +165,18 @@ static void gdb_rename(CPUState *cs, 
> gdb_syscall_complete_cb complete,
>  gdb_do_syscall(complete, "rename,%s,%s", oname, olen, nname, nlen);
>  }
>  
> +static void gdb_system(CPUState *cs, gdb_syscall_complete_cb complete,
> +   target_ulong cmd, target_ulong cmd_len)
> +{
> +int len = validate_strlen(cs, cmd, cmd_len);
> +if (len < 0) {
> +complete(cs, -1, -len);
> +return;
> +}
> +
> +gdb_do_syscall(complete, "system,%s", cmd, len);
> +}
> +
>  /*
>   * Host semihosting syscall implementations.
>   */
> @@ -353,6 +365,24 @@ static void host_rename(CPUState *cs, 
> gdb_syscall_complete_cb complete,
>  unlock_user(nstr, nname, 0);
>  }
>  
> +static void host_system(CPUState *cs, gdb_syscall_complete_cb complete,
> +target_ulong cmd, target_ulong cmd_len)
> +{
> +CPUArchState *env G_GNUC_UNUSED = cs->env_ptr;
> +char *p;
> +int ret;
> +
> +ret = validate_lock_user_string(&p, cs, cmd, cmd_len);
> +if (ret < 0) {
> +complete(cs, -1, -ret);
> +return;
> +}
> +
> +ret = system(p);
> +complete(cs, ret, ret == -1 ? errno : 0);
> +unlock_user(p, cmd, 0);
> +}
> +
>  /*
>   * Static file semihosting syscall implementations.
>   */
> @@ -619,3 +649,13 @@ void semihost_sys_rename(CPUState *cs, 
> gdb_syscall_complete_cb complete,
>  host_rename(cs, complete, oname, oname_len, nname, nname_len);
>  }
>  }
> +
> +void semihost_sys_system(CPUState *cs, gdb_syscall_complete_cb complete,
> + target_ulong cmd, target_ulong cmd_len)
> +{
> +if (use_gdb_syscalls()) {
> +gdb_system(cs, complete, cmd, cmd_len);
> +} else {
> +host_system(cs, complete, cmd, cmd_len);
> +}
> +}
> -- 
> 2.34.1
> 







[PATCH v4 37/53] semihosting: Split out semihost_sys_system

2022-06-07 Thread Richard Henderson
Split out the non-ARM specific portions of SYS_SYSTEM to a
reusable function.

Signed-off-by: Richard Henderson 
---
 include/semihosting/syscalls.h |  3 +++
 semihosting/arm-compat-semi.c  | 12 +-
 semihosting/syscalls.c | 40 ++
 3 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/include/semihosting/syscalls.h b/include/semihosting/syscalls.h
index 21430aa0ef..c9f9e66be1 100644
--- a/include/semihosting/syscalls.h
+++ b/include/semihosting/syscalls.h
@@ -56,4 +56,7 @@ 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);
 
+void semihost_sys_system(CPUState *cs, gdb_syscall_complete_cb complete,
+ target_ulong cmd, target_ulong cmd_len);
+
 #endif /* SEMIHOSTING_SYSCALLS_H */
diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
index da5c80b3d5..8a4e22a20a 100644
--- a/semihosting/arm-compat-semi.c
+++ b/semihosting/arm-compat-semi.c
@@ -506,17 +506,7 @@ void do_common_semihosting(CPUState *cs)
 case TARGET_SYS_SYSTEM:
 GET_ARG(0);
 GET_ARG(1);
-if (use_gdb_syscalls()) {
-gdb_do_syscall(common_semi_cb, "system,%s", arg0, (int)arg1 + 1);
-break;
-}
-s = lock_user_string(arg0);
-if (!s) {
-goto do_fault;
-}
-ret = system(s);
-unlock_user(s, arg0, 0);
-common_semi_cb(cs, ret, ret == -1 ? errno : 0);
+semihost_sys_system(cs, common_semi_cb, arg0, arg1 + 1);
 break;
 
 case TARGET_SYS_ERRNO:
diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c
index 223916b110..de846ced32 100644
--- a/semihosting/syscalls.c
+++ b/semihosting/syscalls.c
@@ -165,6 +165,18 @@ static void gdb_rename(CPUState *cs, 
gdb_syscall_complete_cb complete,
 gdb_do_syscall(complete, "rename,%s,%s", oname, olen, nname, nlen);
 }
 
+static void gdb_system(CPUState *cs, gdb_syscall_complete_cb complete,
+   target_ulong cmd, target_ulong cmd_len)
+{
+int len = validate_strlen(cs, cmd, cmd_len);
+if (len < 0) {
+complete(cs, -1, -len);
+return;
+}
+
+gdb_do_syscall(complete, "system,%s", cmd, len);
+}
+
 /*
  * Host semihosting syscall implementations.
  */
@@ -353,6 +365,24 @@ static void host_rename(CPUState *cs, 
gdb_syscall_complete_cb complete,
 unlock_user(nstr, nname, 0);
 }
 
+static void host_system(CPUState *cs, gdb_syscall_complete_cb complete,
+target_ulong cmd, target_ulong cmd_len)
+{
+CPUArchState *env G_GNUC_UNUSED = cs->env_ptr;
+char *p;
+int ret;
+
+ret = validate_lock_user_string(&p, cs, cmd, cmd_len);
+if (ret < 0) {
+complete(cs, -1, -ret);
+return;
+}
+
+ret = system(p);
+complete(cs, ret, ret == -1 ? errno : 0);
+unlock_user(p, cmd, 0);
+}
+
 /*
  * Static file semihosting syscall implementations.
  */
@@ -619,3 +649,13 @@ void semihost_sys_rename(CPUState *cs, 
gdb_syscall_complete_cb complete,
 host_rename(cs, complete, oname, oname_len, nname, nname_len);
 }
 }
+
+void semihost_sys_system(CPUState *cs, gdb_syscall_complete_cb complete,
+ target_ulong cmd, target_ulong cmd_len)
+{
+if (use_gdb_syscalls()) {
+gdb_system(cs, complete, cmd, cmd_len);
+} else {
+host_system(cs, complete, cmd, cmd_len);
+}
+}
-- 
2.34.1