From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KpSRX-00024e-6I for qemu-devel@nongnu.org; Mon, 13 Oct 2008 14:46:59 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KpSRW-00024B-Mi for qemu-devel@nongnu.org; Mon, 13 Oct 2008 14:46:58 -0400 Received: from [199.232.76.173] (port=46993 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KpSRW-000242-7b for qemu-devel@nongnu.org; Mon, 13 Oct 2008 14:46:58 -0400 Received: from nf-out-0910.google.com ([64.233.182.185]:45952) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KpSRV-0007zT-MB for qemu-devel@nongnu.org; Mon, 13 Oct 2008 14:46:58 -0400 Received: by nf-out-0910.google.com with SMTP id b2so947912nfb.12 for ; Mon, 13 Oct 2008 11:46:56 -0700 (PDT) Date: Mon, 13 Oct 2008 21:48:09 +0300 From: "Kirill A. Shutemov" Subject: Re: [Qemu-devel] [PATCH] Fix and cleanup IPCOP_msg* ipc calls handling Message-ID: <20081013184804.GA21226@localhost.localdomain> References: <1223892640-15545-1-git-send-email-kirill@shutemov.name> <1223892640-15545-2-git-send-email-kirill@shutemov.name> <1223892640-15545-3-git-send-email-kirill@shutemov.name> <20081013155311.GA14963@volta.aurel32.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="OXfL5xGRrasGEqWY" Content-Disposition: inline In-Reply-To: <20081013155311.GA14963@volta.aurel32.net> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: qemu-devel@nongnu.org --OXfL5xGRrasGEqWY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Oct 13, 2008 at 05:53:11PM +0200, Aurelien Jarno wrote: > On Mon, Oct 13, 2008 at 01:10:30PM +0300, Kirill A. Shutemov wrote: > > Signed-off-by: Kirill A. Shutemov > > --- > > linux-user/syscall.c | 173 ++++++++++++++++++++++++++++++++++--------= -------- > > 1 files changed, 117 insertions(+), 56 deletions(-) >=20 > Please find my comments below. In general, please avoid mixing > indentation changes with code changes. This only makes the code more > difficult to review. Ok. By the way, what is correct indentation options for qemu? I configure my vi= m=20 with softwidth=3D4 and set expandtab. Is it correct? > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > > index 40e985a..7e67093 100644 > > --- a/linux-user/syscall.c > > +++ b/linux-user/syscall.c > > @@ -1611,7 +1611,6 @@ static abi_long do_socketcall(int num, abi_ulong = vptr) > > } > > #endif > > =20 > > -#ifdef TARGET_NR_ipc > > #define N_SHM_REGIONS 32 > > =20 > > static struct shm_region { > > @@ -1845,20 +1844,26 @@ static inline abi_long do_semctl(int first, int= second, int third, > > =20 > > struct target_msqid_ds > > { > > - struct target_ipc_perm msg_perm; > > - abi_ulong msg_stime; > > - abi_ulong __unused1; > > - abi_ulong msg_rtime; > > - abi_ulong __unused2; > > - abi_ulong msg_ctime; > > - abi_ulong __unused3; > > - abi_ulong __msg_cbytes; > > - abi_ulong msg_qnum; > > - abi_ulong msg_qbytes; > > - abi_ulong msg_lspid; > > - abi_ulong msg_lrpid; > > - abi_ulong __unused4; > > - abi_ulong __unused5; > > + struct target_ipc_perm msg_perm; > > + abi_ulong msg_stime; > > +#if TARGET_ABI_BITS =3D=3D 32 > > + abi_ulong __unused1; > > +#endif > > + abi_ulong msg_rtime; > > +#if TARGET_ABI_BITS =3D=3D 32 > > + abi_ulong __unused2; > > +#endif > > + abi_ulong msg_ctime; > > +#if TARGET_ABI_BITS =3D=3D 32 > > + abi_ulong __unused3; > > +#endif >=20 > Could you explain me why those __unused* are only present with > TARGET_ABI_BITS? They needed to align following filds to 64-bit on 32-bit hosts. Since on 64-bit hosts sizeof(long) =3D=3D 8, it's unneeded. > This is not consistent with the kernel interface > defined in the glibc. Really?=20 glibc 2.5.1, /usr/include/bits/msq.h: 38 struct msqid_ds 39 { 40 struct ipc_perm msg_perm; /* structure describing operation permi 41 __time_t msg_stime; /* time of last msgsnd command */ 42 #if __WORDSIZE =3D=3D 32 43 unsigned long int __unused1; 44 #endif 45 __time_t msg_rtime; /* time of last msgrcv command */ 46 #if __WORDSIZE =3D=3D 32 47 unsigned long int __unused2; 48 #endif 49 __time_t msg_ctime; /* time of last change */ 50 #if __WORDSIZE =3D=3D 32 51 unsigned long int __unused3; 52 #endif 53 unsigned long int __msg_cbytes; /* current number of bytes on queue * 54 msgqnum_t msg_qnum; /* number of messages currently on queu 55 msglen_t msg_qbytes; /* max number of bytes allowed on queue 56 __pid_t msg_lspid; /* pid of last msgsnd() */ 57 __pid_t msg_lrpid; /* pid of last msgrcv() */ 58 unsigned long int __unused4; 59 unsigned long int __unused5; 60 }; > > + abi_ulong __msg_cbytes; > > + abi_ulong msg_qnum; > > + abi_ulong msg_qbytes; > > + abi_ulong msg_lspid; > > + abi_ulong msg_lrpid; > > + abi_ulong __unused4; > > + abi_ulong __unused5; > > }; =20 > > +static inline abi_long do_msgctl(int msgid, int cmd, abi_long ptr) > > { > > struct msqid_ds dsarg; > > - int cmd =3D second&0xff; > > - abi_long ret =3D 0; > > - switch( cmd ) { > > + struct msginfo msginfo; > > + abi_long ret =3D -TARGET_EINVAL; > > + > > + cmd &=3D 0xff; > > + > > + switch (cmd) { > > case IPC_STAT: > > case IPC_SET: > > - target_to_host_msqid_ds(&dsarg,ptr); > > - ret =3D get_errno(msgctl(first, cmd, &dsarg)); > > - host_to_target_msqid_ds(ptr,&dsarg); > > - default: > > - ret =3D get_errno(msgctl(first, cmd, &dsarg)); >=20 > How is default handled now? Since we handle all valid cmd, we just return -TARGET_EINVAL. --=20 Regards, Kirill A. Shutemov + Belarus, Minsk + ALT Linux Team, http://www.altlinux.com/ --OXfL5xGRrasGEqWY Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) iEYEARECAAYFAkjzl+QACgkQbWYnhzC5v6pYkgCfZ5ExIS+8SLZfVxMU9T+JEAOA on0AnA7ThNUUA0QN+vFh07lpa2A5zxQE =GHOu -----END PGP SIGNATURE----- --OXfL5xGRrasGEqWY--