From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58753) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TDEaK-0008Lz-99 for qemu-devel@nongnu.org; Sun, 16 Sep 2012 09:08:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TDEaI-00031R-Uu for qemu-devel@nongnu.org; Sun, 16 Sep 2012 09:08:28 -0400 Received: from cantor2.suse.de ([195.135.220.15]:50946 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TDEaI-00031F-J1 for qemu-devel@nongnu.org; Sun, 16 Sep 2012 09:08:26 -0400 Message-ID: <5055CF46.5060603@suse.de> Date: Sun, 16 Sep 2012 15:08:22 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1347740649-28646-1-git-send-email-rth@twiddle.net> <1347740649-28646-7-git-send-email-rth@twiddle.net> In-Reply-To: <1347740649-28646-7-git-send-email-rth@twiddle.net> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 6/6] linux-user: Fix siginfo handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: Riku Voipio , qemu-devel@nongnu.org Am 15.09.2012 22:24, schrieb Richard Henderson: > Compare signal numbers in the proper domain. > Convert all of the fields for SIGIO and SIGCHLD. >=20 > Signed-off-by: Richard Henderson > --- > linux-user/qemu.h | 3 +++ > linux-user/signal.c | 59 +++++++++++++++++++++++++++++++++++---------= -------- > linux-user/syscall.c | 2 +- > 3 files changed, 44 insertions(+), 20 deletions(-) >=20 > diff --git a/linux-user/qemu.h b/linux-user/qemu.h > index 69b27d7..8f871eb 100644 > --- a/linux-user/qemu.h > +++ b/linux-user/qemu.h > @@ -219,6 +219,9 @@ unsigned long init_guest_space(unsigned long host_s= tart, > =20 > #include "qemu-log.h" > =20 > +/* syscall.c */ > +int host_to_target_waitstatus(int status); > + > /* strace.c */ > void print_syscall(int num, > abi_long arg1, abi_long arg2, abi_long arg3, > diff --git a/linux-user/signal.c b/linux-user/signal.c > index bf2dfb8..9842ba6 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -202,46 +202,67 @@ void target_to_host_old_sigset(sigset_t *sigset, > static inline void host_to_target_siginfo_noswap(target_siginfo_t *tin= fo, > const siginfo_t *info= ) > { > - int sig; > - sig =3D host_to_target_signal(info->si_signo); > + int sig =3D host_to_target_signal(info->si_signo); > tinfo->si_signo =3D sig; > tinfo->si_errno =3D 0; > tinfo->si_code =3D info->si_code; > - if (sig =3D=3D SIGILL || sig =3D=3D SIGFPE || sig =3D=3D SIGSEGV |= | > - sig =3D=3D SIGBUS || sig =3D=3D SIGTRAP) { > - /* should never come here, but who knows. The information for > - the target is irrelevant */ > + > + if (sig =3D=3D TARGET_SIGILL || sig =3D=3D TARGET_SIGFPE || sig =3D= =3D TARGET_SIGSEGV > + || sig =3D=3D TARGET_SIGBUS || sig =3D=3D TARGET_SIGTRAP) { > + /* Should never come here, but who knows. The information for > + the target is irrelevant. */ > tinfo->_sifields._sigfault._addr =3D 0; > - } else if (sig =3D=3D SIGIO) { > + } else if (sig =3D=3D TARGET_SIGIO) { > + tinfo->_sifields._sigpoll._band =3D info->si_band; > tinfo->_sifields._sigpoll._fd =3D info->si_fd; > + } else if (sig =3D=3D TARGET_SIGCHLD) { > + tinfo->_sifields._sigchld._pid =3D info->si_pid; > + tinfo->_sifields._sigchld._uid =3D info->si_uid; > + tinfo->_sifields._sigchld._status > + =3D host_to_target_waitstatus(info->si_status); > + tinfo->_sifields._sigchld._utime =3D info->si_utime; > + tinfo->_sifields._sigchld._stime =3D info->si_stime; > } else if (sig >=3D TARGET_SIGRTMIN) { > tinfo->_sifields._rt._pid =3D info->si_pid; > tinfo->_sifields._rt._uid =3D info->si_uid; > /* XXX: potential problem if 64 bit */ > - tinfo->_sifields._rt._sigval.sival_ptr =3D > - (abi_ulong)(unsigned long)info->si_value.sival_ptr; > + tinfo->_sifields._rt._sigval.sival_ptr > + =3D (abi_ulong)(unsigned long)info->si_value.sival_ptr; I don't spot any functional change here ... > } > } > =20 > static void tswap_siginfo(target_siginfo_t *tinfo, > const target_siginfo_t *info) > { > - int sig; > - sig =3D info->si_signo; > + int sig =3D info->si_signo; > tinfo->si_signo =3D tswap32(sig); > tinfo->si_errno =3D tswap32(info->si_errno); > tinfo->si_code =3D tswap32(info->si_code); > - if (sig =3D=3D SIGILL || sig =3D=3D SIGFPE || sig =3D=3D SIGSEGV |= | > - sig =3D=3D SIGBUS || sig =3D=3D SIGTRAP) { > - tinfo->_sifields._sigfault._addr =3D > - tswapal(info->_sifields._sigfault._addr); > - } else if (sig =3D=3D SIGIO) { > - tinfo->_sifields._sigpoll._fd =3D tswap32(info->_sifields._sigpoll._f= d); > + > + if (sig =3D=3D TARGET_SIGILL || sig =3D=3D TARGET_SIGFPE || sig =3D= =3D TARGET_SIGSEGV > + || sig =3D=3D TARGET_SIGBUS || sig =3D=3D TARGET_SIGTRAP) { > + tinfo->_sifields._sigfault._addr > + =3D tswapal(info->_sifields._sigfault._addr); > + } else if (sig =3D=3D TARGET_SIGIO) { > + tinfo->_sifields._sigpoll._band > + =3D tswap32(info->_sifields._sigpoll._band); > + tinfo->_sifields._sigpoll._fd =3D tswap32(info->_sifields._sig= poll._fd); > + } else if (sig =3D=3D TARGET_SIGCHLD) { > + tinfo->_sifields._sigchld._pid > + =3D tswap32(info->_sifields._sigchld._pid); > + tinfo->_sifields._sigchld._uid > + =3D tswap32(info->_sifields._sigchld._uid); > + tinfo->_sifields._sigchld._status > + =3D tswap32(info->_sifields._sigchld._status); > + tinfo->_sifields._sigchld._utime > + =3D tswapal(info->_sifields._sigchld._utime); > + tinfo->_sifields._sigchld._stime > + =3D tswapal(info->_sifields._sigchld._stime); > } else if (sig >=3D TARGET_SIGRTMIN) { > tinfo->_sifields._rt._pid =3D tswap32(info->_sifields._rt._pid= ); > tinfo->_sifields._rt._uid =3D tswap32(info->_sifields._rt._uid= ); > - tinfo->_sifields._rt._sigval.sival_ptr =3D > - tswapal(info->_sifields._rt._sigval.sival_ptr); > + tinfo->_sifields._rt._sigval.sival_ptr > + =3D tswapal(info->_sifields._rt._sigval.sival_ptr); ... or here. Does Coding Style mandate this? Would be nice to separate from functional changes then or to leave out. Otherwise no functional issue spotted, using TARGET_SIG* after conversion certainly makes sense. :) Andreas > } > } > =20 > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 925e579..3676c72 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -4920,7 +4920,7 @@ static int do_futex(target_ulong uaddr, int op, i= nt val, target_ulong timeout, > =20 > /* Map host to target signal numbers for the wait family of syscalls. > Assume all other status bits are the same. */ > -static int host_to_target_waitstatus(int status) > +int host_to_target_waitstatus(int status) > { > if (WIFSIGNALED(status)) { > return host_to_target_signal(WTERMSIG(status)) | (status & ~0x= 7f); >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg