On Wed, 23 Apr 2014 19:00:41 +0100 Peter Maydell <peter.mayd...@linaro.org> wrote:
> 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? 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 <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... 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