From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Kggfl-0006rJ-Er for qemu-devel@nongnu.org; Fri, 19 Sep 2008 10:09:25 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Kggfk-0006qW-Tl for qemu-devel@nongnu.org; Fri, 19 Sep 2008 10:09:25 -0400 Received: from [199.232.76.173] (port=57187 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Kggfk-0006qT-Qe for qemu-devel@nongnu.org; Fri, 19 Sep 2008 10:09:24 -0400 Received: from [84.20.150.76] (port=58332 helo=narury.org) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Kggfk-0004Rs-3L for qemu-devel@nongnu.org; Fri, 19 Sep 2008 10:09:24 -0400 Date: Fri, 19 Sep 2008 17:09:18 +0300 From: Riku Voipio Subject: Re: [Qemu-devel] [PATCH] Implement fstatat64() syscall Message-ID: <20080919140918.GD21479@kos.to> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1221750426-14863-5-git-send-email-kirill@shutemov.name> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: "Kirill A. Shutemov" 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. > > Signed-off-by: Kirill A. Shutemov > --- > linux-user/syscall.c | 141 +++++++++++++++++++++++++++++-------------------- > 1 files changed, 83 insertions(+), 58 deletions(-) > > 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 == 32 > _syscall3(int, sys_getdents, uint, fd, struct dirent *, dirp, uint, count); > @@ -3149,6 +3154,67 @@ static inline abi_long host_to_target_timespec(abi_ulong target_addr, > return 0; > } > > +#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 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. > + > #if defined(USE_NPTL) > /* ??? Using host futex calls even when target atomic operations > are not really atomic probably breaks things. However implementing > @@ -5154,7 +5220,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, > goto efault; > ret = get_errno(stat(path(p), &st)); > unlock_user(p, arg1, 0); > - goto do_stat64; > + if (!is_error(ret)) > + ret = host_to_target_stat64(cpu_env, arg2, &st); > + break; > #endif > #ifdef TARGET_NR_lstat64 > case TARGET_NR_lstat64: > @@ -5162,67 +5230,24 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, > goto efault; > ret = get_errno(lstat(path(p), &st)); > unlock_user(p, arg1, 0); > - goto do_stat64; > + if (!is_error(ret)) > + ret = host_to_target_stat64(cpu_env, arg2, &st); > + break; > #endif > #ifdef TARGET_NR_fstat64 > case TARGET_NR_fstat64: > - { > - ret = get_errno(fstat(arg1, &st)); > - do_stat64: > - if (!is_error(ret)) { > -#ifdef TARGET_ARM > - if (((CPUARMState *)cpu_env)->eabi) { > - struct target_eabi_stat64 *target_st; > - > - if (!lock_user_struct(VERIFY_WRITE, target_st, arg2, 0)) > - goto efault; > - memset(target_st, 0, sizeof(struct target_eabi_stat64)); > - __put_user(st.st_dev, &target_st->st_dev); > - __put_user(st.st_ino, &target_st->st_ino); > -#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO > - __put_user(st.st_ino, &target_st->__st_ino); > -#endif > - __put_user(st.st_mode, &target_st->st_mode); > - __put_user(st.st_nlink, &target_st->st_nlink); > - __put_user(st.st_uid, &target_st->st_uid); > - __put_user(st.st_gid, &target_st->st_gid); > - __put_user(st.st_rdev, &target_st->st_rdev); > - __put_user(st.st_size, &target_st->st_size); > - __put_user(st.st_blksize, &target_st->st_blksize); > - __put_user(st.st_blocks, &target_st->st_blocks); > - __put_user(st.st_atime, &target_st->target_st_atime); > - __put_user(st.st_mtime, &target_st->target_st_mtime); > - __put_user(st.st_ctime, &target_st->target_st_ctime); > - unlock_user_struct(target_st, arg2, 1); > - } else > + ret = get_errno(fstat(arg1, &st)); > + if (!is_error(ret)) > + ret = host_to_target_stat64(cpu_env, arg2, &st); > + break; > #endif > - { > - struct target_stat64 *target_st; > - > - if (!lock_user_struct(VERIFY_WRITE, target_st, arg2, 0)) > - goto efault; > - memset(target_st, 0, sizeof(struct target_stat64)); > - __put_user(st.st_dev, &target_st->st_dev); > - __put_user(st.st_ino, &target_st->st_ino); > -#ifdef TARGET_STAT64_HAS_BROKEN_ST_INO > - __put_user(st.st_ino, &target_st->__st_ino); > -#endif > - __put_user(st.st_mode, &target_st->st_mode); > - __put_user(st.st_nlink, &target_st->st_nlink); > - __put_user(st.st_uid, &target_st->st_uid); > - __put_user(st.st_gid, &target_st->st_gid); > - __put_user(st.st_rdev, &target_st->st_rdev); > - /* XXX: better use of kernel struct */ > - __put_user(st.st_size, &target_st->st_size); > - __put_user(st.st_blksize, &target_st->st_blksize); > - __put_user(st.st_blocks, &target_st->st_blocks); > - __put_user(st.st_atime, &target_st->target_st_atime); > - __put_user(st.st_mtime, &target_st->target_st_mtime); > - __put_user(st.st_ctime, &target_st->target_st_ctime); > - unlock_user_struct(target_st, arg2, 1); > - } > - } > - } > +#if defined(TARGET_NR_fstatat64) && defined(__NR_fstatat64) > + case TARGET_NR_fstatat64: > + if (!(p = lock_user_string(arg2))) > + goto efault; > + ret = get_errno(sys_fstatat64(arg1, path(p), &st, arg4)); > + if (!is_error(ret)) > + ret = host_to_target_stat64(cpu_env, arg3, &st); > break; > #endif > #ifdef USE_UID16 > -- > 1.5.6.5.GIT > > -- "rm -rf" only sounds scary if you don't have backups