Re: [Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined
Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20171220002941.14560-1-pal...@dabbelt.com Subject: [Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total=$(git log --oneline $BASE.. | wc -l) failed=0 git config --local diff.renamelimit 0 git config --local diff.renames True commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then failed=1 echo fi n=$((n+1)) done exit $failed === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' bcff5a4829 linux-user: Implement renameat2 when defined === OUTPUT BEGIN === Checking PATCH 1/1: linux-user: Implement renameat2 when defined... WARNING: architecture specific defines should be avoided #57: FILE: linux-user/syscall.c:8345: +#if defined(TARGET_NR_renameat2) && defined(__NR_renameat2) ERROR: braces {} are necessary for all arms of this statement #63: FILE: linux-user/syscall.c:8351: +if (!p || !p2) [...] +else [...] WARNING: line over 80 characters #66: FILE: linux-user/syscall.c:8354: +ret = get_errno(syscall(__NR_renameat2, arg1, p, arg3, p2, arg5)); total: 1 errors, 2 warnings, 21 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@freelists.org
Re: [Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined
On Thu, 21 Dec 2017 06:01:25 PST (-0800), peter.mayd...@linaro.org wrote: On 20 December 2017 at 00:29, Palmer Dabbelt wrote: +#if defined(TARGET_NR_renameat2) && defined(__NR_renameat2) +case TARGET_NR_renameat2: +{ +void *p2; +p = lock_user_string(arg2); +p2 = lock_user_string(arg4); +if (!p || !p2) +ret = -TARGET_EFAULT; +else +ret = get_errno(syscall(__NR_renameat2, arg1, p, arg3, p2, arg5)); +unlock_user(p2, arg4, 0); +unlock_user(p, arg2, 0); +} +break; +#endif Should we have code to handle using plain renameat for flags==0 calls? renameat2() only arrived in 3.15 kernels, so as it stands this patch will work on a smaller set of hosts than it might. That sound sane: there isn't even a glibc wrapper for renameat2, so I assume most calls will be with flags==0. Do you want me to re-spin the patch, or can you take it from here?
Re: [Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined
On 20 December 2017 at 00:29, Palmer Dabbelt wrote: > +#if defined(TARGET_NR_renameat2) && defined(__NR_renameat2) > +case TARGET_NR_renameat2: > +{ > +void *p2; > +p = lock_user_string(arg2); > +p2 = lock_user_string(arg4); > +if (!p || !p2) > +ret = -TARGET_EFAULT; > +else > +ret = get_errno(syscall(__NR_renameat2, arg1, p, arg3, p2, > arg5)); > +unlock_user(p2, arg4, 0); > +unlock_user(p, arg2, 0); > +} > +break; > +#endif Should we have code to handle using plain renameat for flags==0 calls? renameat2() only arrived in 3.15 kernels, so as it stands this patch will work on a smaller set of hosts than it might. thanks -- PMM
Re: [Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined
On 12/20/2017 01:29 AM, Palmer Dabbelt wrote: > From: Palmer Dabbelt > > The RISC-V Linux port was recently accept upstream and will be released > as part of 4.15. While working on our glibc port I discovered that > qemu's user-mode emulation doesn't support renameat2, which has replaced > rename as part of the default system call list for new architectures. > Since a bunch of commonly used functionality boils down to rename (and > now renameat2), we ended up with many failures. > > This patch adds support for renameat2. As I'm not familiar with QEMU > development, I haven't really testing anything more than a simple > "./configure; make" on the upstream codebase, but I did test this > against our (not yet upstream) QEMU port where it appears to work for > me. I've just cobbled it together by copying the existing renameat > implementation, but as there appears to be no glibc wrapper for > renameat2 on either of the systems I've tried this on I just emited the > system call directly. > CC'ed linux-user maintainer Riku Voipio. Cheers, Bastian
[Qemu-devel] [PATCH] linux-user: Implement renameat2 when defined
From: Palmer Dabbelt The RISC-V Linux port was recently accept upstream and will be released as part of 4.15. While working on our glibc port I discovered that qemu's user-mode emulation doesn't support renameat2, which has replaced rename as part of the default system call list for new architectures. Since a bunch of commonly used functionality boils down to rename (and now renameat2), we ended up with many failures. This patch adds support for renameat2. As I'm not familiar with QEMU development, I haven't really testing anything more than a simple "./configure; make" on the upstream codebase, but I did test this against our (not yet upstream) QEMU port where it appears to work for me. I've just cobbled it together by copying the existing renameat implementation, but as there appears to be no glibc wrapper for renameat2 on either of the systems I've tried this on I just emited the system call directly. commit b0da6d44157aa6e652de7634343708251ba64146 Author: James Hogan Date: Fri Apr 29 22:29:26 2016 +0100 asm-generic: Drop renameat syscall from default list The newer renameat2 syscall provides all the functionality provided by the renameat syscall and adds flags, so future architectures won't need to include renameat. Therefore drop the renameat syscall from the generic syscall list unless __ARCH_WANT_RENAMEAT is defined by the architecture's unistd.h prior to including asm-generic/unistd.h, and adjust all architectures using the generic syscall list to define it so that no in-tree architectures are affected. Michael, Sagar, and Bastian are the RISC-V QEMU guys, I just use it. I've CC'd them all here to serve as sort of a surprise introduction for everyone -- they haven't seen the patch before, so blame me for the stupid parts :). CC: Michael Clark CC: Sagar Karandikar CC: Bastian Koppelmann Signed-off-by: Palmer Dabbelt --- linux-user/syscall.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 11c9116c4a1e..ece574f47aca 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -8342,6 +8342,21 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, } break; #endif +#if defined(TARGET_NR_renameat2) && defined(__NR_renameat2) +case TARGET_NR_renameat2: +{ +void *p2; +p = lock_user_string(arg2); +p2 = lock_user_string(arg4); +if (!p || !p2) +ret = -TARGET_EFAULT; +else +ret = get_errno(syscall(__NR_renameat2, arg1, p, arg3, p2, arg5)); +unlock_user(p2, arg4, 0); +unlock_user(p, arg2, 0); +} +break; +#endif #ifdef TARGET_NR_mkdir case TARGET_NR_mkdir: if (!(p = lock_user_string(arg1))) -- 2.13.6