On 23 April 2014 15:59, Natanael Copa <nc...@alpinelinux.org> wrote:
> Avoid using glibc specific internals.
>
> Calculate the sigevent pad size is calculated in similar way as kernel
> does it.
>
> This is needed for building with musl libc.
Thanks for this patch -- is this the only fix that was needed, or
are there more to come?
It would be nice to be a little more specific in the patch summary
line about the change, like:
linux-user: avoid using glibc internals in definition of
target_sigevent struct
> Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
> ---
> linux-user/syscall.c | 2 +-
> linux-user/syscall_defs.h | 6 +++++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9864813..c8989b6 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -406,7 +406,7 @@ static int sys_inotify_init1(int flags)
> #endif
> #define __NR_sys_ppoll __NR_ppoll
> _syscall5(int, sys_ppoll, struct pollfd *, fds, nfds_t, nfds,
> - struct timespec *, timeout, const __sigset_t *, sigmask,
> + struct timespec *, timeout, const sigset_t *, sigmask,
> size_t, sigsetsize)
> #endif
>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index fdf9a47..450f71b 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -2552,12 +2552,16 @@ struct target_timer_t {
> abi_ulong ptr;
> };
>
> +#define TARGET_SIGEV_MAX_SIZE 64
> +#define TARGET_SIGEV_PREAMABLE_SIZE (sizeof(int32_t) * 2 +
> sizeof(target_sigval_t))
This is wrong for 64 bit MIPS, I think; compare the kernel's
MIPS-specific override:
http://lxr.linux.no/#linux+v3.14.1/arch/mips/include/uapi/asm/siginfo.h#L13
I suggest
/* This is architecture-specific but most architectures use the default */
#ifdef TARGET_MIPS
#define TARGET_SIGEV_PREAMBLE_SIZE (sizeof(int32_t) * 2 + sizeof(abi_long))
#else
TARGET_SIGEV_PREAMBLE_SIZE (sizeof(int32_t) * 2 + sizeof(target_sigval_t))
#endif
I'm not entirely sure this is required -- our target_sigval_t looks
like it ought to be sizeof(abi_long) for MIPS so I don't know
why the kernel needs this override and we apparently don't.
Perhaps our target_sigval_t definition is slightly wrong?
Anyway, putting in the override can't hurt and might avoid subtle
issues later on if target_sigval_t does turn out to be broken and
need changing...
(Note also that 'PREAMBLE' only has one 'A'.)
> +#define TARGET_SIGEV_PAD_SIZE ((TARGET_SIGEV_MAX_SIZE -
> TARGET_SIGEV_PREAMABLE_SIZE) / sizeof(int32_t))
This line looks like it has more than 80 chars; if so, it should be
folded. (You can check using scripts/checkpatch.pl.)
> struct target_sigevent {
> target_sigval_t sigev_value;
> int32_t sigev_signo;
> int32_t sigev_notify;
> union {
> - int32_t _pad[ARRAY_SIZE(((struct sigevent *)0)->_sigev_un._pad)];
> + int32_t _pad[TARGET_SIGEV_PAD_SIZE];
> int32_t _tid;
>
> struct {
> --
> 1.9.2
Looks good overall though.
thanks
-- PMM
