From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Kggtx-0003fU-Dy for qemu-devel@nongnu.org; Fri, 19 Sep 2008 10:24:05 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Kggtv-0003ea-TF for qemu-devel@nongnu.org; Fri, 19 Sep 2008 10:24:04 -0400 Received: from [199.232.76.173] (port=47019 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Kggtv-0003eW-HX for qemu-devel@nongnu.org; Fri, 19 Sep 2008 10:24:03 -0400 Received: from nf-out-0910.google.com ([64.233.182.185]:42260) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Kggtu-0008Op-2l for qemu-devel@nongnu.org; Fri, 19 Sep 2008 10:24:02 -0400 Received: by nf-out-0910.google.com with SMTP id b2so234299nfb.12 for ; Fri, 19 Sep 2008 07:23:55 -0700 (PDT) Date: Fri, 19 Sep 2008 17:24:40 +0300 From: "Kirill A. Shutemov" Subject: Re: [Qemu-devel] [PATCH] Implement fstatat64() syscall Message-ID: <20080919142439.GH5346@localhost.localdomain> References: <1221750426-14863-1-git-send-email-kirill@shutemov.name> <1221750426-14863-2-git-send-email-kirill@shutemov.name> <1221750426-14863-3-git-send-email-kirill@shutemov.name> <1221750426-14863-4-git-send-email-kirill@shutemov.name> <1221750426-14863-5-git-send-email-kirill@shutemov.name> <20080919140918.GD21479@kos.to> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="cW+P/jduATWpL925" Content-Disposition: inline In-Reply-To: <20080919140918.GD21479@kos.to> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Riku Voipio Cc: qemu-devel@nongnu.org --cW+P/jduATWpL925 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 19, 2008 at 05:09:18PM +0300, Riku Voipio wrote: > On Thu, Sep 18, 2008 at 06:07:03PM +0300, Kirill A. Shutemov wrote: > > Move transformation of struct stat64 into the separate function and > > implement fstatat64() using it. > >=20 > > Signed-off-by: Kirill A. Shutemov > > --- > > linux-user/syscall.c | 141 +++++++++++++++++++++++++++++-------------= ------- > > 1 files changed, 83 insertions(+), 58 deletions(-) > >=20 > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > > index 88b44b8..ac7e7d9 100644 > > --- a/linux-user/syscall.c > > +++ b/linux-user/syscall.c > > @@ -156,6 +156,7 @@ static type name (type1 arg1,type2 arg2,type3 arg3,= type4 arg4,type5 arg5, \ > > #define __NR_sys_faccessat __NR_faccessat > > #define __NR_sys_fchmodat __NR_fchmodat > > #define __NR_sys_fchownat __NR_fchownat > > +#define __NR_sys_fstatat64 __NR_fstatat64 > > #define __NR_sys_getcwd1 __NR_getcwd > > #define __NR_sys_getdents __NR_getdents > > #define __NR_sys_getdents64 __NR_getdents64 > > @@ -200,6 +201,10 @@ _syscall4(int,sys_fchmodat,int,dirfd,const char *,= pathname, > > _syscall5(int,sys_fchownat,int,dirfd,const char *,pathname, > > uid_t,owner,gid_t,group,int,flags) > > #endif > > +#if defined(TARGET_NR_fstatat64) && defined(__NR_fstatat64) > > +_syscall4(int,sys_fstatat64,int,dirfd,const char *,pathname, > > + struct stat *,buf,int,flags) > > +#endif > > _syscall2(int,sys_getcwd1,char *,buf,size_t,size) > > #if TARGET_ABI_BITS =3D=3D 32 > > _syscall3(int, sys_getdents, uint, fd, struct dirent *, dirp, uint, co= unt); > > @@ -3149,6 +3154,67 @@ static inline abi_long host_to_target_timespec(a= bi_ulong target_addr, > > return 0; > > } > > =20 > > +#ifdef TARGET_NR_stat64 > > +static inline abi_long host_to_target_stat64(void *cpu_env, > > + abi_ulong target_addr, > > + struct stat *host_st) > > +{ > > +#ifdef TARGET_ARM > > + if (((CPUARMState *)cpu_env)->eabi) { > > + struct target_eabi_stat64 *target_st; > > + > > + if (!lock_user_struct(VERIFY_WRITE, target_st, target_addr, 0)) > > + return -TARGET_EFAULT; > > + memset(target_st, 0, sizeof(struct target_eabi_stat64)); > > + __put_user(host_st->st_dev, &target_st->st_dev); > > + __put_user(host_st->st_ino, &target_st->st_ino); > > +#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO > > + __put_user(host_st->st_ino, &target_st->__st_ino); > > +#endif > > + __put_user(host_st->st_mode, &target_st->st_mode); > > + __put_user(host_st->st_nlink, &target_st->st_nlink); > > + __put_user(host_st->st_uid, &target_st->st_uid); > > + __put_user(host_st->st_gid, &target_st->st_gid); > > + __put_user(host_st->st_rdev, &target_st->st_rdev); > > + __put_user(host_st->st_size, &target_st->st_size); > > + __put_user(host_st->st_blksize, &target_st->st_blksize); > > + __put_user(host_st->st_blocks, &target_st->st_blocks); > > + __put_user(host_st->st_atime, &target_st->target_st_atime); > > + __put_user(host_st->st_mtime, &target_st->target_st_mtime); > > + __put_user(host_st->st_ctime, &target_st->target_st_ctime); > > + unlock_user_struct(target_st, target_addr, 1); > > + } else > > +#endif > > + { > > + struct target_stat64 *target_st; > > + > > + if (!lock_user_struct(VERIFY_WRITE, target_st, target_addr, 0)) > > + return -TARGET_EFAULT; > > + memset(target_st, 0, sizeof(struct target_stat64)); > > + __put_user(host_st->st_dev, &target_st->st_dev); > > + __put_user(host_st->st_ino, &target_st->st_ino); > > +#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO > > + __put_user(host_st->st_ino, &target_st->__st_ino); > > +#endif > > + __put_user(host_st->st_mode, &target_st->st_mode); > > + __put_user(host_st->st_nlink, &target_st->st_nlink); > > + __put_user(host_st->st_uid, &target_st->st_uid); > > + __put_user(host_st->st_gid, &target_st->st_gid); > > + __put_user(host_st->st_rdev, &target_st->st_rdev); > > + /* XXX: better use of kernel struct */ > > + __put_user(host_st->st_size, &target_st->st_size); > > + __put_user(host_st->st_blksize, &target_st->st_blksize); > > + __put_user(host_st->st_blocks, &target_st->st_blocks); > > + __put_user(host_st->st_atime, &target_st->target_st_atime); > > + __put_user(host_st->st_mtime, &target_st->target_st_mtime); > > + __put_user(host_st->st_ctime, &target_st->target_st_ctime); > > + unlock_user_struct(target_st, target_addr, 1); > > + } > > + > > + return 0; > > +} > > +#endif >=20 > This is suboptimal - we same code (list of __put_user()) twice. We > should have smaller if/else in the beginning of the function that sets > target_st. Pay attention that struct in 'if' and in 'else' is different. There is no way to make it pretty(without dirty preprocessing hacks). --=20 Regards, Kirill A. Shutemov + Belarus, Minsk + ALT Linux Team, http://www.altlinux.com/ --cW+P/jduATWpL925 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) iEYEARECAAYFAkjTticACgkQbWYnhzC5v6rK8wCdERNphgE5muAvHNKasPugayU+ H0EAoIzPe+LdmP+qfQ0hJdc81kccuJPt =CISX -----END PGP SIGNATURE----- --cW+P/jduATWpL925--