[Qemu-devel] Wshadow in qemu/linux-user/syscall.c

ghorges posted 1 patch 5 years ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch failed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190408084056.26212-1-ghorges@xiyoulinux.org
Maintainers: Riku Voipio <riku.voipio@iki.fi>, Laurent Vivier <laurent@vivier.eu>
linux-user/syscall.c | 66 ++++++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 33 deletions(-)
[Qemu-devel] Wshadow in qemu/linux-user/syscall.c
Posted by ghorges 5 years ago
---
 linux-user/syscall.c | 66 ++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 208fd1813d..985095e4d5 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8240,7 +8240,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
                 size_t size;
             } sig, *sig_ptr;
 
-            abi_ulong arg_sigset, arg_sigsize, *arg7;
+            abi_ulong arg_sigset, arg_sigsize, *arg7s;
             target_sigset_t *target_sigset;
 
             n = arg1;
@@ -8280,13 +8280,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
                 sig_ptr = &sig;
                 sig.size = SIGSET_T_SIZE;
 
-                arg7 = lock_user(VERIFY_READ, arg6, sizeof(*arg7) * 2, 1);
-                if (!arg7) {
+                arg7s = lock_user(VERIFY_READ, arg6, sizeof(*arg7s) * 2, 1);
+                if (!arg7s) {
                     return -TARGET_EFAULT;
                 }
-                arg_sigset = tswapal(arg7[0]);
-                arg_sigsize = tswapal(arg7[1]);
-                unlock_user(arg7, arg6, 0);
+                arg_sigset = tswapal(arg7s[0]);
+                arg_sigsize = tswapal(arg7s[1]);
+                unlock_user(arg7s, arg6, 0);
 
                 if (arg_sigset) {
                     sig.set = &set;
@@ -9479,14 +9479,14 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
         }
     case TARGET_NR_getcpu:
         {
-            unsigned cpu, node;
-            ret = get_errno(sys_getcpu(arg1 ? &cpu : NULL,
+            unsigned cpus, node;
+            ret = get_errno(sys_getcpu(arg1 ? &cpus : NULL,
                                        arg2 ? &node : NULL,
                                        NULL));
             if (is_error(ret)) {
                 return ret;
             }
-            if (arg1 && put_user_u32(cpu, arg1)) {
+            if (arg1 && put_user_u32(cpus, arg1)) {
                 return -TARGET_EFAULT;
             }
             if (arg2 && put_user_u32(node, arg2)) {
@@ -10649,24 +10649,24 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_listxattr:
     case TARGET_NR_llistxattr:
     {
-        void *p, *b = 0;
+        void *q, *b = 0;
         if (arg2) {
             b = lock_user(VERIFY_WRITE, arg2, arg3, 0);
             if (!b) {
                 return -TARGET_EFAULT;
             }
         }
-        p = lock_user_string(arg1);
-        if (p) {
+        q = lock_user_string(arg1);
+        if (q) {
             if (num == TARGET_NR_listxattr) {
-                ret = get_errno(listxattr(p, b, arg3));
+                ret = get_errno(listxattr(q, b, arg3));
             } else {
-                ret = get_errno(llistxattr(p, b, arg3));
+                ret = get_errno(llistxattr(q, b, arg3));
             }
         } else {
             ret = -TARGET_EFAULT;
         }
-        unlock_user(p, arg1, 0);
+        unlock_user(q, arg1, 0);
         unlock_user(b, arg2, arg3);
         return ret;
     }
@@ -10686,25 +10686,25 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_setxattr:
     case TARGET_NR_lsetxattr:
         {
-            void *p, *n, *v = 0;
+            void *q, *n, *v = 0;
             if (arg3) {
                 v = lock_user(VERIFY_READ, arg3, arg4, 1);
                 if (!v) {
                     return -TARGET_EFAULT;
                 }
             }
-            p = lock_user_string(arg1);
+            q = lock_user_string(arg1);
             n = lock_user_string(arg2);
-            if (p && n) {
+            if (q && n) {
                 if (num == TARGET_NR_setxattr) {
-                    ret = get_errno(setxattr(p, n, v, arg4, arg5));
+                    ret = get_errno(setxattr(q, n, v, arg4, arg5));
                 } else {
-                    ret = get_errno(lsetxattr(p, n, v, arg4, arg5));
+                    ret = get_errno(lsetxattr(q, n, v, arg4, arg5));
                 }
             } else {
                 ret = -TARGET_EFAULT;
             }
-            unlock_user(p, arg1, 0);
+            unlock_user(q, arg1, 0);
             unlock_user(n, arg2, 0);
             unlock_user(v, arg3, 0);
         }
@@ -10731,25 +10731,25 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_getxattr:
     case TARGET_NR_lgetxattr:
         {
-            void *p, *n, *v = 0;
+            void *q, *n, *v = 0;
             if (arg3) {
                 v = lock_user(VERIFY_WRITE, arg3, arg4, 0);
                 if (!v) {
                     return -TARGET_EFAULT;
                 }
             }
-            p = lock_user_string(arg1);
+            q = lock_user_string(arg1);
             n = lock_user_string(arg2);
-            if (p && n) {
+            if (q && n) {
                 if (num == TARGET_NR_getxattr) {
-                    ret = get_errno(getxattr(p, n, v, arg4));
+                    ret = get_errno(getxattr(q, n, v, arg4));
                 } else {
-                    ret = get_errno(lgetxattr(p, n, v, arg4));
+                    ret = get_errno(lgetxattr(q, n, v, arg4));
                 }
             } else {
                 ret = -TARGET_EFAULT;
             }
-            unlock_user(p, arg1, 0);
+            unlock_user(q, arg1, 0);
             unlock_user(n, arg2, 0);
             unlock_user(v, arg3, arg4);
         }
@@ -10776,19 +10776,19 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_removexattr:
     case TARGET_NR_lremovexattr:
         {
-            void *p, *n;
-            p = lock_user_string(arg1);
+            void *q, *n;
+            q = lock_user_string(arg1);
             n = lock_user_string(arg2);
-            if (p && n) {
+            if (q && n) {
                 if (num == TARGET_NR_removexattr) {
-                    ret = get_errno(removexattr(p, n));
+                    ret = get_errno(removexattr(q, n));
                 } else {
-                    ret = get_errno(lremovexattr(p, n));
+                    ret = get_errno(lremovexattr(q, n));
                 }
             } else {
                 ret = -TARGET_EFAULT;
             }
-            unlock_user(p, arg1, 0);
+            unlock_user(q, arg1, 0);
             unlock_user(n, arg2, 0);
         }
         return ret;
-- 
2.17.0





Re: [Qemu-devel] Wshadow in qemu/linux-user/syscall.c
Posted by no-reply@patchew.org 5 years ago
Patchew URL: https://patchew.org/QEMU/20190408084056.26212-1-ghorges@xiyoulinux.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20190408084056.26212-1-ghorges@xiyoulinux.org
Subject: [Qemu-devel] Wshadow in qemu/linux-user/syscall.c
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20190405064121.23662-1-richardw.yang@linux.intel.com -> patchew/20190405064121.23662-1-richardw.yang@linux.intel.com
 t [tag update]            patchew/20190405153314.2068-1-philmd@redhat.com -> patchew/20190405153314.2068-1-philmd@redhat.com
 * [new tag]               patchew/20190408084056.26212-1-ghorges@xiyoulinux.org -> patchew/20190408084056.26212-1-ghorges@xiyoulinux.org
Switched to a new branch 'test'
c76af142cd Wshadow in qemu/linux-user/syscall.c

=== OUTPUT BEGIN ===
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 160 lines checked

Commit c76af142cd02 (Wshadow in qemu/linux-user/syscall.c) 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


The full log is available at
http://patchew.org/logs/20190408084056.26212-1-ghorges@xiyoulinux.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] Wshadow in qemu/linux-user/syscall.c
Posted by Thomas Huth 5 years ago
 Hi,

thanks for your patch! But please add a short patch description (beside
the subject), e.g. "Change the name of some variables so that the code
can be compiled with -Wshadow, too".

And you've got to put a "Signed-off-by" line at the end of the patch
description, too. See this URL for details:

https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line

For future patches to files in the linux-user directory, please also CC:
the maintainers of this subsystem (i.e. Ruku Voipio and Laurent Vivier,
see MAINTAINERS file).

 Thanks,
  Thomas


On 08/04/2019 10.40, ghorges wrote:
> ---
>  linux-user/syscall.c | 66 ++++++++++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 208fd1813d..985095e4d5 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8240,7 +8240,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>                  size_t size;
>              } sig, *sig_ptr;
>  
> -            abi_ulong arg_sigset, arg_sigsize, *arg7;
> +            abi_ulong arg_sigset, arg_sigsize, *arg7s;
>              target_sigset_t *target_sigset;
>  
>              n = arg1;
> @@ -8280,13 +8280,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>                  sig_ptr = &sig;
>                  sig.size = SIGSET_T_SIZE;
>  
> -                arg7 = lock_user(VERIFY_READ, arg6, sizeof(*arg7) * 2, 1);
> -                if (!arg7) {
> +                arg7s = lock_user(VERIFY_READ, arg6, sizeof(*arg7s) * 2, 1);
> +                if (!arg7s) {
>                      return -TARGET_EFAULT;
>                  }
> -                arg_sigset = tswapal(arg7[0]);
> -                arg_sigsize = tswapal(arg7[1]);
> -                unlock_user(arg7, arg6, 0);
> +                arg_sigset = tswapal(arg7s[0]);
> +                arg_sigsize = tswapal(arg7s[1]);
> +                unlock_user(arg7s, arg6, 0);
>  
>                  if (arg_sigset) {
>                      sig.set = &set;
> @@ -9479,14 +9479,14 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>          }
>      case TARGET_NR_getcpu:
>          {
> -            unsigned cpu, node;
> -            ret = get_errno(sys_getcpu(arg1 ? &cpu : NULL,
> +            unsigned cpus, node;
> +            ret = get_errno(sys_getcpu(arg1 ? &cpus : NULL,
>                                         arg2 ? &node : NULL,
>                                         NULL));
>              if (is_error(ret)) {
>                  return ret;
>              }
> -            if (arg1 && put_user_u32(cpu, arg1)) {
> +            if (arg1 && put_user_u32(cpus, arg1)) {
>                  return -TARGET_EFAULT;
>              }
>              if (arg2 && put_user_u32(node, arg2)) {
> @@ -10649,24 +10649,24 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>      case TARGET_NR_listxattr:
>      case TARGET_NR_llistxattr:
>      {
> -        void *p, *b = 0;
> +        void *q, *b = 0;
>          if (arg2) {
>              b = lock_user(VERIFY_WRITE, arg2, arg3, 0);
>              if (!b) {
>                  return -TARGET_EFAULT;
>              }
>          }
> -        p = lock_user_string(arg1);
> -        if (p) {
> +        q = lock_user_string(arg1);
> +        if (q) {
>              if (num == TARGET_NR_listxattr) {
> -                ret = get_errno(listxattr(p, b, arg3));
> +                ret = get_errno(listxattr(q, b, arg3));
>              } else {
> -                ret = get_errno(llistxattr(p, b, arg3));
> +                ret = get_errno(llistxattr(q, b, arg3));
>              }
>          } else {
>              ret = -TARGET_EFAULT;
>          }
> -        unlock_user(p, arg1, 0);
> +        unlock_user(q, arg1, 0);
>          unlock_user(b, arg2, arg3);
>          return ret;
>      }
> @@ -10686,25 +10686,25 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>      case TARGET_NR_setxattr:
>      case TARGET_NR_lsetxattr:
>          {
> -            void *p, *n, *v = 0;
> +            void *q, *n, *v = 0;
>              if (arg3) {
>                  v = lock_user(VERIFY_READ, arg3, arg4, 1);
>                  if (!v) {
>                      return -TARGET_EFAULT;
>                  }
>              }
> -            p = lock_user_string(arg1);
> +            q = lock_user_string(arg1);
>              n = lock_user_string(arg2);
> -            if (p && n) {
> +            if (q && n) {
>                  if (num == TARGET_NR_setxattr) {
> -                    ret = get_errno(setxattr(p, n, v, arg4, arg5));
> +                    ret = get_errno(setxattr(q, n, v, arg4, arg5));
>                  } else {
> -                    ret = get_errno(lsetxattr(p, n, v, arg4, arg5));
> +                    ret = get_errno(lsetxattr(q, n, v, arg4, arg5));
>                  }
>              } else {
>                  ret = -TARGET_EFAULT;
>              }
> -            unlock_user(p, arg1, 0);
> +            unlock_user(q, arg1, 0);
>              unlock_user(n, arg2, 0);
>              unlock_user(v, arg3, 0);
>          }
> @@ -10731,25 +10731,25 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>      case TARGET_NR_getxattr:
>      case TARGET_NR_lgetxattr:
>          {
> -            void *p, *n, *v = 0;
> +            void *q, *n, *v = 0;
>              if (arg3) {
>                  v = lock_user(VERIFY_WRITE, arg3, arg4, 0);
>                  if (!v) {
>                      return -TARGET_EFAULT;
>                  }
>              }
> -            p = lock_user_string(arg1);
> +            q = lock_user_string(arg1);
>              n = lock_user_string(arg2);
> -            if (p && n) {
> +            if (q && n) {
>                  if (num == TARGET_NR_getxattr) {
> -                    ret = get_errno(getxattr(p, n, v, arg4));
> +                    ret = get_errno(getxattr(q, n, v, arg4));
>                  } else {
> -                    ret = get_errno(lgetxattr(p, n, v, arg4));
> +                    ret = get_errno(lgetxattr(q, n, v, arg4));
>                  }
>              } else {
>                  ret = -TARGET_EFAULT;
>              }
> -            unlock_user(p, arg1, 0);
> +            unlock_user(q, arg1, 0);
>              unlock_user(n, arg2, 0);
>              unlock_user(v, arg3, arg4);
>          }
> @@ -10776,19 +10776,19 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>      case TARGET_NR_removexattr:
>      case TARGET_NR_lremovexattr:
>          {
> -            void *p, *n;
> -            p = lock_user_string(arg1);
> +            void *q, *n;
> +            q = lock_user_string(arg1);
>              n = lock_user_string(arg2);
> -            if (p && n) {
> +            if (q && n) {
>                  if (num == TARGET_NR_removexattr) {
> -                    ret = get_errno(removexattr(p, n));
> +                    ret = get_errno(removexattr(q, n));
>                  } else {
> -                    ret = get_errno(lremovexattr(p, n));
> +                    ret = get_errno(lremovexattr(q, n));
>                  }
>              } else {
>                  ret = -TARGET_EFAULT;
>              }
> -            unlock_user(p, arg1, 0);
> +            unlock_user(q, arg1, 0);
>              unlock_user(n, arg2, 0);
>          }
>          return ret;
> 


Re: [Qemu-devel] Wshadow in qemu/linux-user/syscall.c
Posted by Kevin Wolf 5 years ago
Am 08.04.2019 um 10:40 hat ghorges geschrieben:
> ---
>  linux-user/syscall.c | 66 ++++++++++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 33 deletions(-)

Thomas already gave you some hints what to improve. I'd just like to add
that you shouldn't send the email to the qemu-block mailing list if the
patch is not about the block layer at all.

Kevin

Re: [Qemu-devel] Wshadow in qemu/linux-user/syscall.c
Posted by 马艺诚 5 years ago
Sorry, I am a newbie. I have never used this kind of thing before. 


I will definitely pay attention next time.
 
------------------ Original ------------------
From:  "Kevin Wolf"<kwolf@redhat.com>;
Date:  Mon, Apr 8, 2019 06:39 PM
To:  "ghorges"<ghorges@xiyoulinux.org>; 
Cc:  "qemu-devel"<qemu-devel@nongnu.org>; "qemu-block"<qemu-block@nongnu.org>; 
Subject:  Re: [Qemu-devel] Wshadow in qemu/linux-user/syscall.c

 

Am 08.04.2019 um 10:40 hat ghorges geschrieben:
> ---
>  linux-user/syscall.c | 66 ++++++++++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 33 deletions(-)

Thomas already gave you some hints what to improve. I'd just like to add
that you shouldn't send the email to the qemu-block mailing list if the
patch is not about the block layer at all.

Kevin