This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 25/29] [AARCH64] Add kernel_sigaction.h for AARCH64 ILP32
- From: Will Newton <will dot newton at linaro dot org>
- To: Andrew Pinski <apinski at cavium dot com>
- Cc: libc-alpha <libc-alpha at sourceware dot org>
- Date: Tue, 18 Nov 2014 14:42:21 +0000
- Subject: Re: [PATCH 25/29] [AARCH64] Add kernel_sigaction.h for AARCH64 ILP32
- Authentication-results: sourceware.org; auth=none
- References: <1414396793-9005-1-git-send-email-apinski at cavium dot com> <1414396793-9005-26-git-send-email-apinski at cavium dot com>
On 27 October 2014 07:59, Andrew Pinski <apinski@cavium.com> wrote:
> In ILP32, the sigaction struct is the same as AARCH64 so we need
> the header file kernel_sigaction.h. To allow for this to work,
> we use a long long fields and then add extra casts when converting
> between the user exposed struct and the kernel exposed struct.
>
> * sysdeps/unix/sysv/linux/aarch64/kernel_sigaction.h: New file.
> * sysdeps/unix/sysv/linux/aarch64/sigaction.c (__libc_sigaction):
> Add cast here it is necessary.
Add casts where necessary.
> ---
> sysdeps/unix/sysv/linux/aarch64/kernel_sigaction.h | 12 ++++++++++++
> sysdeps/unix/sysv/linux/aarch64/sigaction.c | 10 ++++++----
> 2 files changed, 18 insertions(+), 4 deletions(-)
> create mode 100644 sysdeps/unix/sysv/linux/aarch64/kernel_sigaction.h
>
> diff --git a/sysdeps/unix/sysv/linux/aarch64/kernel_sigaction.h b/sysdeps/unix/sysv/linux/aarch64/kernel_sigaction.h
> new file mode 100644
> index 0000000..7b3023b
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/aarch64/kernel_sigaction.h
> @@ -0,0 +1,12 @@
> +
> +#define HAVE_SA_RESTORER
> +
> +/* This is the sigaction structure in aarch64 kernel.
> + Note the ILP32 struct uses the same struct as LP64
> + which is why the fields are 64bit in size. */
Two spaces before end of comment.
> +struct kernel_sigaction {
> + unsigned long long k_sa_handler;
> + unsigned long long sa_flags;
> + unsigned long long sa_restorer;
> + sigset_t sa_mask;
> +};
> diff --git a/sysdeps/unix/sysv/linux/aarch64/sigaction.c b/sysdeps/unix/sysv/linux/aarch64/sigaction.c
> index ae6c3fd..8adcbba 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/sigaction.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/sigaction.c
> @@ -39,15 +39,17 @@ __libc_sigaction (int sig, const struct sigaction *act, struct sigaction *oact)
>
> if (act)
> {
> - kact.k_sa_handler = act->sa_handler;
> + kact.k_sa_handler = (unsigned long long)(uintptr_t)act->sa_handler;
> memcpy (&kact.sa_mask, &act->sa_mask, sizeof (sigset_t));
> kact.sa_flags = act->sa_flags;
> #ifdef HAVE_SA_RESTORER
> if (kact.sa_flags & SA_RESTORER)
> - kact.sa_restorer = act->sa_restorer;
> + kact.sa_restorer = (unsigned long long)(uintptr_t)act->sa_restorer;
> #endif
> }
>
> + /* This is needed for ILP32 as the structures are two different sizes due to
> + using the LP64 structure. */
It's not clear to me what this comment refers to.
> result = INLINE_SYSCALL (rt_sigaction, 4, sig,
> act ? &kact : NULL,
> oact ? &koact : NULL, _NSIG / 8);
> @@ -55,11 +57,11 @@ __libc_sigaction (int sig, const struct sigaction *act, struct sigaction *oact)
> {
> if (oact && result >= 0)
> {
> - oact->sa_handler = koact.k_sa_handler;
> + oact->sa_handler = (void*)(uintptr_t)koact.k_sa_handler;
> memcpy (&oact->sa_mask, &koact.sa_mask, sizeof (sigset_t));
> oact->sa_flags = koact.sa_flags;
> #ifdef HAVE_SA_RESTORER
> - oact->sa_restorer = koact.sa_restorer;
> + oact->sa_restorer = (void*)(uintptr_t)koact.sa_restorer;
Space before * and after casts.
> #endif
> }
> }
> --
> 1.7.2.5
>
Otherwise looks ok.
--
Will Newton
Toolchain Working Group, Linaro