Re: [PATCH v2 2/5] linux-user: Move tswap_siginfo out of target code

[Alex Bennée]

Gustavo Romero <gustavo.romero@linaro.org> writes:

> Move tswap_siginfo from target code to handle_pending_signal. This will
> allow some cleanups and having the siginfo ready to be used in gdbstub.
>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/aarch64/signal.c     |  2 +-
>  linux-user/alpha/signal.c       |  2 +-
>  linux-user/arm/signal.c         |  2 +-
>  linux-user/hexagon/signal.c     |  2 +-
>  linux-user/hppa/signal.c        |  2 +-
>  linux-user/i386/signal.c        |  6 +++---
>  linux-user/loongarch64/signal.c |  2 +-
>  linux-user/m68k/signal.c        |  4 ++--
>  linux-user/microblaze/signal.c  |  2 +-
>  linux-user/mips/signal.c        |  4 ++--
>  linux-user/nios2/signal.c       |  2 +-
>  linux-user/openrisc/signal.c    |  2 +-
>  linux-user/ppc/signal.c         |  4 ++--
>  linux-user/riscv/signal.c       |  2 +-
>  linux-user/s390x/signal.c       |  2 +-
>  linux-user/sh4/signal.c         |  2 +-
>  linux-user/signal-common.h      |  2 --
>  linux-user/signal.c             | 10 ++++++++--
>  linux-user/sparc/signal.c       |  2 +-
>  linux-user/xtensa/signal.c      |  2 +-
>  20 files changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
<snip>
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -409,8 +409,8 @@ static inline void 
> host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
>      tinfo->si_code = deposit32(si_code, 16, 16, si_type);
>  }
>  
> -void tswap_siginfo(target_siginfo_t *tinfo,
> -                   const target_siginfo_t *info)
> +static void tswap_siginfo(target_siginfo_t *tinfo,
> +                          const target_siginfo_t *info)
>  {
>      int si_type = extract32(info->si_code, 16, 16);
>      int si_code = sextract32(info->si_code, 0, 16);
> @@ -1180,6 +1180,12 @@ static void handle_pending_signal(CPUArchState 
> *cpu_env, int sig,
>      /* dequeue signal */
>      k->pending = 0;
>  
> +    /*
> +     * Writes out siginfo values byteswapped, accordingly to the target. It 
> also
> +     * cleans the si_type from si_code making it correct for the target.
> +     */
> +    tswap_siginfo(&k->info, &k->info);
> +

I'm not sure I like this, you have the same pointer to both a const and
non-const arg. Do we assert we come through this once per signal and
don't risk double swapping the contents?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro