* Re: [Qemu-devel] [PATCH] linux-user: implement renameat2
2018-01-22 17:44 [Qemu-devel] [PATCH] linux-user: implement renameat2 Andreas Schwab
@ 2018-01-22 17:48 ` no-reply
2018-01-23 0:35 ` Laurent Vivier
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: no-reply @ 2018-01-22 17:48 UTC (permalink / raw)
To: schwab; +Cc: famz, qemu-devel
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: mvmk1w9lreq.fsf@suse.de
Subject: [Qemu-devel] [PATCH] linux-user: implement renameat2
=== 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
From https://github.com/patchew-project/qemu
t [tag update] patchew/1516611841-5526-1-git-send-email-anton.nefedov@virtuozzo.com -> patchew/1516611841-5526-1-git-send-email-anton.nefedov@virtuozzo.com
* [new tag] patchew/mvmk1w9lreq.fsf@suse.de -> patchew/mvmk1w9lreq.fsf@suse.de
Switched to a new branch 'test'
59d79798ac linux-user: implement renameat2
=== OUTPUT BEGIN ===
Checking PATCH 1/1: linux-user: implement renameat2...
WARNING: architecture specific defines should be avoided
#21: FILE: linux-user/syscall.c:602:
+#if defined(__NR_renameat2)
ERROR: spaces required around that '*' (ctx:WxV)
#27: FILE: linux-user/syscall.c:608:
+ int newfd, const chat *new, int flags)
^
ERROR: braces {} are necessary for all arms of this statement
#29: FILE: linux-user/syscall.c:610:
+ if (flags == 0)
[...]
ERROR: braces {} are necessary for all arms of this statement
#50: FILE: linux-user/syscall.c:8368:
+ if (!p || !p2)
[...]
+ else
[...]
ERROR: line over 90 characters
#54: FILE: linux-user/syscall.c:8372:
+ target_to_host_bitmask(arg5, fcntl_flags_tbl)));
total: 4 errors, 1 warnings, 45 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-devel@freelists.org
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH] linux-user: implement renameat2
2018-01-22 17:44 [Qemu-devel] [PATCH] linux-user: implement renameat2 Andreas Schwab
2018-01-22 17:48 ` no-reply
@ 2018-01-23 0:35 ` Laurent Vivier
2018-01-23 10:22 ` Andreas Schwab
2018-01-23 0:37 ` Laurent Vivier
2018-01-23 10:53 ` [Qemu-devel] [PATCH v2] " Andreas Schwab
3 siblings, 1 reply; 9+ messages in thread
From: Laurent Vivier @ 2018-01-23 0:35 UTC (permalink / raw)
To: Andreas Schwab, qemu-devel
Le 22/01/2018 à 18:44, Andreas Schwab a écrit :
> This is needed for new architectures like RISC-V which do not provide any
> other rename-like syscall.
>
> Signed-off-by: Andreas Schwab <schwab@suse.de>
> ---
> linux-user/syscall.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 5e54889522..12ca06c65a 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -598,6 +598,23 @@ static int sys_utimensat(int dirfd, const char *pathname,
> #endif
> #endif /* TARGET_NR_utimensat */
>
> +#ifdef TARGET_NR_renameat2
> +#if defined(__NR_renameat2)
> +#define __NR_sys_renameat2 __NR_renameat2
> +_syscall5(int, sys_renameat2, int, oldfd, const char *, old, int, newfd,
> + const char *, new, unsigned int, flags)
> +#else
> +static int sys_renameat2(int oldfd, const char *old,
> + int newfd, const chat *new, int flags)
> +{
> + if (flags == 0)
> + return renameat(oldfd, old, newfd, new);
Please fix style problem reported by patchew
(or ./scripts/checkpatch.pl)
> + errno = ENOSYS;
> + return -1;
> +}
> +#endif
> +#endif /* TARGET_NR_renameat2 */
> +
> #ifdef CONFIG_INOTIFY
> #include <sys/inotify.h>
>
> @@ -8342,6 +8359,22 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> }
> break;
> #endif
> +#if defined(TARGET_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(sys_renameat2(arg1, p, arg3, p2,
> + target_to_host_bitmask(arg5, fcntl_flags_tbl)));
You can't use fcntl_flags_tbl
(because it converts fcntl/open flags O_*).
I think you can provide directly arg5 to sys_renameat2() because flags
are the same for all architectures (and value is already byte-swapped).
Thanks,
Laurent
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH] linux-user: implement renameat2
2018-01-23 0:35 ` Laurent Vivier
@ 2018-01-23 10:22 ` Andreas Schwab
0 siblings, 0 replies; 9+ messages in thread
From: Andreas Schwab @ 2018-01-23 10:22 UTC (permalink / raw)
To: Laurent Vivier; +Cc: qemu-devel
On Jan 23 2018, Laurent Vivier <laurent@vivier.eu> wrote:
> Please fix style problem reported by patchew
> (or ./scripts/checkpatch.pl)
This was mostly copy-pasted from surrounding code. :-)
>> @@ -8342,6 +8359,22 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>> }
>> break;
>> #endif
>> +#if defined(TARGET_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(sys_renameat2(arg1, p, arg3, p2,
>> + target_to_host_bitmask(arg5, fcntl_flags_tbl)));
>
> You can't use fcntl_flags_tbl
Of course! I was confused by another patch I was working on which does
need such a conversion. :-/
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: implement renameat2
2018-01-22 17:44 [Qemu-devel] [PATCH] linux-user: implement renameat2 Andreas Schwab
2018-01-22 17:48 ` no-reply
2018-01-23 0:35 ` Laurent Vivier
@ 2018-01-23 0:37 ` Laurent Vivier
2018-01-23 10:26 ` Andreas Schwab
2018-01-23 10:53 ` [Qemu-devel] [PATCH v2] " Andreas Schwab
3 siblings, 1 reply; 9+ messages in thread
From: Laurent Vivier @ 2018-01-23 0:37 UTC (permalink / raw)
To: Andreas Schwab, qemu-devel
Le 22/01/2018 à 18:44, Andreas Schwab a écrit :
> This is needed for new architectures like RISC-V which do not provide any
> other rename-like syscall.
>
> Signed-off-by: Andreas Schwab <schwab@suse.de>
> ---
> linux-user/syscall.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 5e54889522..12ca06c65a 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -598,6 +598,23 @@ static int sys_utimensat(int dirfd, const char *pathname,
> #endif
> #endif /* TARGET_NR_utimensat */
>
> +#ifdef TARGET_NR_renameat2
> +#if defined(__NR_renameat2)
> +#define __NR_sys_renameat2 __NR_renameat2
> +_syscall5(int, sys_renameat2, int, oldfd, const char *, old, int, newfd,
> + const char *, new, unsigned int, flags)
> +#else
> +static int sys_renameat2(int oldfd, const char *old,
> + int newfd, const chat *new, int flags)
"const char *" would look better.
And, please, test it (build/run).
Thanks,
Laurent
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: implement renameat2
2018-01-23 0:37 ` Laurent Vivier
@ 2018-01-23 10:26 ` Andreas Schwab
2018-01-23 10:53 ` Laurent Vivier
0 siblings, 1 reply; 9+ messages in thread
From: Andreas Schwab @ 2018-01-23 10:26 UTC (permalink / raw)
To: Laurent Vivier; +Cc: qemu-devel
On Jan 23 2018, Laurent Vivier <laurent@vivier.eu> wrote:
> And, please, test it (build/run).
This was tested by bootstrapping openSUSE Factory for RISC-V.
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: implement renameat2
2018-01-23 10:26 ` Andreas Schwab
@ 2018-01-23 10:53 ` Laurent Vivier
0 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2018-01-23 10:53 UTC (permalink / raw)
To: qemu-devel
Le 23/01/2018 à 11:26, Andreas Schwab a écrit :
> On Jan 23 2018, Laurent Vivier <laurent@vivier.eu> wrote:
>
>> And, please, test it (build/run).
>
> This was tested by bootstrapping openSUSE Factory for RISC-V.
I have no doubt on this part, but you should test the "#else" part too,
by, for instance, undefining "__NR_renameat2" locally/temporarily.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v2] linux-user: implement renameat2
2018-01-22 17:44 [Qemu-devel] [PATCH] linux-user: implement renameat2 Andreas Schwab
` (2 preceding siblings ...)
2018-01-23 0:37 ` Laurent Vivier
@ 2018-01-23 10:53 ` Andreas Schwab
2018-01-23 11:20 ` Laurent Vivier
3 siblings, 1 reply; 9+ messages in thread
From: Andreas Schwab @ 2018-01-23 10:53 UTC (permalink / raw)
To: qemu-devel
This is needed for new architectures like RISC-V which do not provide any
other rename-like syscall.
Signed-off-by: Andreas Schwab <schwab@suse.de>
---
linux-user/syscall.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5e54889522..c258a0a44b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -598,6 +598,24 @@ static int sys_utimensat(int dirfd, const char *pathname,
#endif
#endif /* TARGET_NR_utimensat */
+#ifdef TARGET_NR_renameat2
+#if defined(__NR_renameat2)
+#define __NR_sys_renameat2 __NR_renameat2
+_syscall5(int, sys_renameat2, int, oldfd, const char *, old, int, newfd,
+ const char *, new, unsigned int, flags)
+#else
+static int sys_renameat2(int oldfd, const char *old,
+ int newfd, const char *new, int flags)
+{
+ if (flags == 0) {
+ return renameat(oldfd, old, newfd, new);
+ }
+ errno = ENOSYS;
+ return -1;
+}
+#endif
+#endif /* TARGET_NR_renameat2 */
+
#ifdef CONFIG_INOTIFY
#include <sys/inotify.h>
@@ -8342,6 +8360,22 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
}
break;
#endif
+#if defined(TARGET_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(sys_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.16.1
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v2] linux-user: implement renameat2
2018-01-23 10:53 ` [Qemu-devel] [PATCH v2] " Andreas Schwab
@ 2018-01-23 11:20 ` Laurent Vivier
0 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2018-01-23 11:20 UTC (permalink / raw)
To: Andreas Schwab, qemu-devel
Le 23/01/2018 à 11:53, Andreas Schwab a écrit :
> This is needed for new architectures like RISC-V which do not provide any
> other rename-like syscall.
>
> Signed-off-by: Andreas Schwab <schwab@suse.de>
> ---
> linux-user/syscall.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 5e54889522..c258a0a44b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -598,6 +598,24 @@ static int sys_utimensat(int dirfd, const char *pathname,
> #endif
> #endif /* TARGET_NR_utimensat */
>
> +#ifdef TARGET_NR_renameat2
> +#if defined(__NR_renameat2)
> +#define __NR_sys_renameat2 __NR_renameat2
> +_syscall5(int, sys_renameat2, int, oldfd, const char *, old, int, newfd,
> + const char *, new, unsigned int, flags)
> +#else
> +static int sys_renameat2(int oldfd, const char *old,
> + int newfd, const char *new, int flags)
> +{
> + if (flags == 0) {
> + return renameat(oldfd, old, newfd, new);
> + }
> + errno = ENOSYS;
> + return -1;
> +}
> +#endif
> +#endif /* TARGET_NR_renameat2 */
> +
> #ifdef CONFIG_INOTIFY
> #include <sys/inotify.h>
>
> @@ -8342,6 +8360,22 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> }
> break;
> #endif
> +#if defined(TARGET_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(sys_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)))
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 9+ messages in thread