Re: [PATCH v2 2/7] linux-user/alpha: Rename the sigaction restorer field

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Use ka_restorer, in line with TARGET_ARCH_HAS_KA_RESTORER
> vs TARGET_ARCH_HAS_SA_RESTORER, since Alpha passes this
> field as a syscall argument.

I'm still slightly confused - but that's to be expected from signals :-/

Anyway I understand that the SA_RESTORER points to the vdso trampoline
(at least according to man sigreturn). What is ka_restorer - if this the
in sigframe restorer?

>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/syscall_defs.h | 2 +-
>  linux-user/alpha/signal.c | 8 ++++----
>  linux-user/syscall.c      | 4 ++--
>  3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 25be414727..693d4f3788 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -519,7 +519,7 @@ struct target_sigaction {
>      abi_ulong _sa_handler;
>      abi_ulong sa_flags;
>      target_sigset_t sa_mask;
> -    abi_ulong sa_restorer;
> +    abi_ulong ka_restorer;
>  };

Maybe this is something we could expand a little on in the difference
between the two here (or maybe in the later commit?).


>  #elif defined(TARGET_MIPS)
>  struct target_sigaction {
> diff --git a/linux-user/alpha/signal.c b/linux-user/alpha/signal.c
> index 86f5d2276d..3aa4b339a4 100644
> --- a/linux-user/alpha/signal.c
> +++ b/linux-user/alpha/signal.c
> @@ -138,8 +138,8 @@ void setup_frame(int sig, struct target_sigaction *ka,
>  
>      setup_sigcontext(&frame->sc, env, frame_addr, set);
>  
> -    if (ka->sa_restorer) {
> -        r26 = ka->sa_restorer;
> +    if (ka->ka_restorer) {
> +        r26 = ka->ka_restorer;
>      } else {
>          __put_user(INSN_MOV_R30_R16, &frame->retcode[0]);
>          __put_user(INSN_LDI_R0 + TARGET_NR_sigreturn,
> @@ -192,8 +192,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>          __put_user(set->sig[i], &frame->uc.tuc_sigmask.sig[i]);
>      }
>  
> -    if (ka->sa_restorer) {
> -        r26 = ka->sa_restorer;
> +    if (ka->ka_restorer) {
> +        r26 = ka->ka_restorer;
>      } else {
>          __put_user(INSN_MOV_R30_R16, &frame->retcode[0]);
>          __put_user(INSN_LDI_R0 + TARGET_NR_rt_sigreturn,
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 95d79ddc43..ee21eb5e6f 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8989,7 +8989,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>                  act._sa_handler = old_act->_sa_handler;
>                  target_siginitset(&act.sa_mask, old_act->sa_mask);
>                  act.sa_flags = old_act->sa_flags;
> -                act.sa_restorer = 0;
> +                act.ka_restorer = 0;
>                  unlock_user_struct(old_act, arg2, 0);
>                  pact = &act;
>              }
> @@ -9085,7 +9085,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>                  act._sa_handler = rt_act->_sa_handler;
>                  act.sa_mask = rt_act->sa_mask;
>                  act.sa_flags = rt_act->sa_flags;
> -                act.sa_restorer = arg5;
> +                act.ka_restorer = arg5;
>                  unlock_user_struct(rt_act, arg2, 0);
>                  pact = &act;
>              }

Otherwise looks fine to me:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée