Re: [Qemu-devel] [PATCH for 2.13 3/5] linux-user: define TARGET_ARCH_HAS_SETUP_FRAME

[Peter Maydell]

On 22 March 2018 at 21:58, Laurent Vivier <address@hidden> wrote:
> instead of calling setup_frame() conditionnaly to a list of knowm targets,

"Instead". "conditionally". "known".

> define TARGET_ARCH_HAS_SETUP_FRAME if the target provides the function
> and call it only if the macro is defined

Trailing ".".

>
> Signed-off-by: Laurent Vivier <address@hidden>
> ---

> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -886,18 +886,13 @@ static void handle_pending_signal(CPUArchState 
> *cpu_env, int sig,
>          }
>  #endif
>          /* prepare the stack frame of the virtual CPU */
> -#if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64) \
> -        || defined(TARGET_OPENRISC) || defined(TARGET_TILEGX) \
> -        || defined(TARGET_PPC64) || defined(TARGET_HPPA) \
> -        || defined(TARGET_NIOS2) || defined(TARGET_X86_64) \
> -        || defined(TARGET_RISCV) || defined(TARGET_XTENSA)
> -        /* These targets do not have traditional signals.  */
> -        setup_rt_frame(sig, sa, &k->info, &target_old_set, cpu_env);
> -#else
> +#if defined(TARGET_ARCH_HAS_SETUP_FRAME)
>          if (sa->sa_flags & TARGET_SA_SIGINFO)
>              setup_rt_frame(sig, sa, &k->info, &target_old_set, cpu_env);
>          else
>              setup_frame(sig, sa, &target_old_set, cpu_env);
> +#else
> +        setup_rt_frame(sig, sa, &k->info, &target_old_set, cpu_env);
>  #endif
>          if (sa->sa_flags & TARGET_SA_RESETHAND) {
>              sa->_sa_handler = TARGET_SIG_DFL;

You might like to add the {} to the if (sa->sa_flags...) while we're
here.

Either way,

Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM