[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] linux-user: avoid using glibc internals
From: |
Natanael Copa |
Subject: |
Re: [Qemu-devel] [PATCH] linux-user: avoid using glibc internals |
Date: |
Tue, 29 Apr 2014 09:24:30 +0200 |
On Wed, 23 Apr 2014 19:00:41 +0100
Peter Maydell <address@hidden> wrote:
> On 23 April 2014 15:59, Natanael Copa <address@hidden> 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?
There are more patches needed to make it build and run with musl libc.
Those were not mine, but I can try clean them up and submit those if
here is interest for it.
The problem this patch resolves was introduced with qemu 2.0.
> 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
Agree.
> > Signed-off-by: Natanael Copa <address@hidden>
> > ---
> > 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...
Ok. No problem.
>
> (Note also that 'PREAMBLE' only has one 'A'.)
whoops.
>
> > +#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.)
I wasn't sure of the folding rules.
> > 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
I'll fix and resend. Thanks for feedback.
-nc