[PATCH 00/10] tests: Clean up global variables shadowing

[PATCH 00/10] tests: Clean up global variables shadowing

[Philippe Mathieu-Daudé]

Clean up global variables shadowing in tests/
in order to be able to use -Wshadow with Clang.

Philippe Mathieu-Daudé (10):
  system/qtest: Clean up global variable shadowing in
    qtest_server_init()
  tests/throttle: Clean up global variable shadowing
  tests/virtio-scsi: Clean up global variable shadowing
  tests/cdrom-test: Clean up global variable shadowing in
    prepare_image()
  tests/hd-geo-test: Clean up global variable shadowing
  tests/rtl8139: Clean up global variable shadowing
  tests/npcm7xx_adc: Clean up global variable shadowing
  tests/aio: Clean up global variable shadowing
  tests/aio-multithread: Clean up global variable shadowing
  tests/coroutine: Clean up global variable shadowing

 softmmu/qtest.c                   | 16 ++++++++--------
 tests/qtest/cdrom-test.c          |  6 +++---
 tests/qtest/hd-geo-test.c         | 12 ++++++++----
 tests/qtest/npcm7xx_adc-test.c    | 14 +++++++-------
 tests/qtest/rtl8139-test.c        | 12 ++++++------
 tests/qtest/virtio-scsi-test.c    |  2 +-
 tests/unit/test-aio-multithread.c | 16 ++++++++--------
 tests/unit/test-aio.c             |  4 ++--
 tests/unit/test-coroutine.c       | 12 ++++++------
 tests/unit/test-throttle.c        |  1 -
 10 files changed, 49 insertions(+), 46 deletions(-)

-- 
2.41.0

[PATCH 01/10] system/qtest: Clean up global variable shadowing in qtest_server_init()

[Philippe Mathieu-Daudé]

Rename the variable to fix:

  softmmu/qtest.c:869:13: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      Object *qtest;
              ^
  softmmu/qtest.c:53:15: note: previous declaration is here
  static QTest *qtest;
                ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 softmmu/qtest.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 35b643a274..7964f0b248 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -866,7 +866,7 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
 {
     ERRP_GUARD();
     Chardev *chr;
-    Object *qtest;
+    Object *qobj;
 
     chr = qemu_chr_new("qtest", qtest_chrdev, NULL);
     if (chr == NULL) {
@@ -875,18 +875,18 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
         return;
     }
 
-    qtest = object_new(TYPE_QTEST);
-    object_property_set_str(qtest, "chardev", chr->label, &error_abort);
+    qobj = object_new(TYPE_QTEST);
+    object_property_set_str(qobj, "chardev", chr->label, &error_abort);
     if (qtest_log) {
-        object_property_set_str(qtest, "log", qtest_log, &error_abort);
+        object_property_set_str(qobj, "log", qtest_log, &error_abort);
     }
-    object_property_add_child(qdev_get_machine(), "qtest", qtest);
-    user_creatable_complete(USER_CREATABLE(qtest), errp);
+    object_property_add_child(qdev_get_machine(), "qtest", qobj);
+    user_creatable_complete(USER_CREATABLE(qobj), errp);
     if (*errp) {
-        object_unparent(qtest);
+        object_unparent(qobj);
     }
     object_unref(OBJECT(chr));
-    object_unref(qtest);
+    object_unref(qobj);
 }
 
 static bool qtest_server_start(QTest *q, Error **errp)
-- 
2.41.0

Re: [PATCH 01/10] system/qtest: Clean up global variable shadowing in qtest_server_init()

[Thomas Huth]

On 09/10/2023 12.02, Philippe Mathieu-Daudé wrote:
> Rename the variable to fix:
> 
>    softmmu/qtest.c:869:13: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>        Object *qtest;
>                ^
>    softmmu/qtest.c:53:15: note: previous declaration is here
>    static QTest *qtest;
>                  ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   softmmu/qtest.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/softmmu/qtest.c b/softmmu/qtest.c
> index 35b643a274..7964f0b248 100644
> --- a/softmmu/qtest.c
> +++ b/softmmu/qtest.c
> @@ -866,7 +866,7 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
>   {
>       ERRP_GUARD();
>       Chardev *chr;
> -    Object *qtest;
> +    Object *qobj;
>   
>       chr = qemu_chr_new("qtest", qtest_chrdev, NULL);
>       if (chr == NULL) {
> @@ -875,18 +875,18 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
>           return;
>       }
>   
> -    qtest = object_new(TYPE_QTEST);
> -    object_property_set_str(qtest, "chardev", chr->label, &error_abort);
> +    qobj = object_new(TYPE_QTEST);
> +    object_property_set_str(qobj, "chardev", chr->label, &error_abort);
>       if (qtest_log) {
> -        object_property_set_str(qtest, "log", qtest_log, &error_abort);
> +        object_property_set_str(qobj, "log", qtest_log, &error_abort);
>       }
> -    object_property_add_child(qdev_get_machine(), "qtest", qtest);
> -    user_creatable_complete(USER_CREATABLE(qtest), errp);
> +    object_property_add_child(qdev_get_machine(), "qtest", qobj);
> +    user_creatable_complete(USER_CREATABLE(qobj), errp);
>       if (*errp) {
> -        object_unparent(qtest);
> +        object_unparent(qobj);
>       }
>       object_unref(OBJECT(chr));
> -    object_unref(qtest);
> +    object_unref(qobj);
>   }

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 02/10] tests/throttle: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Follow all other tests pattern from this file, use the
global 'cfg' variable to fix:

  tests/unit/test-throttle.c:621:20: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      ThrottleConfig cfg;
                     ^
  tests/unit/test-throttle.c:28:23: note: previous declaration is here
  static ThrottleConfig cfg;
                        ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/unit/test-throttle.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
index ac35d65d19..2146cfacd3 100644
--- a/tests/unit/test-throttle.c
+++ b/tests/unit/test-throttle.c
@@ -618,7 +618,6 @@ static bool do_test_accounting(bool is_ops, /* are we testing bps or ops */
                                  { THROTTLE_OPS_TOTAL,
                                    THROTTLE_OPS_READ,
                                    THROTTLE_OPS_WRITE, } };
-    ThrottleConfig cfg;
     BucketType index;
     int i;
 
-- 
2.41.0

Re: [PATCH 02/10] tests/throttle: Clean up global variable shadowing

[Alberto Garcia]

On Mon 09 Oct 2023 12:02:43 PM +02, Philippe Mathieu-Daudé wrote:
> Follow all other tests pattern from this file, use the
> global 'cfg' variable to fix:
>
>   tests/unit/test-throttle.c:621:20: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>       ThrottleConfig cfg;
>                      ^
>   tests/unit/test-throttle.c:28:23: note: previous declaration is here
>   static ThrottleConfig cfg;
>                         ^
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: Alberto Garcia <berto@igalia.com>

Berto

[PATCH 03/10] tests/virtio-scsi: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Rename the (unused) 'allow' argument, following the pattern
used by the other tests in this file. This fixes:

  tests/qtest/virtio-scsi-test.c:159:61: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void hotplug(void *obj, void *data, QGuestAllocator *alloc)
                                                              ^
  tests/qtest/virtio-scsi-test.c:37:25: note: previous declaration is here
  static QGuestAllocator *alloc;
                          ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/virtio-scsi-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/virtio-scsi-test.c b/tests/qtest/virtio-scsi-test.c
index ceaa7f2415..db10d572d0 100644
--- a/tests/qtest/virtio-scsi-test.c
+++ b/tests/qtest/virtio-scsi-test.c
@@ -156,7 +156,7 @@ static QVirtioSCSIQueues *qvirtio_scsi_init(QVirtioDevice *dev)
     return vs;
 }
 
-static void hotplug(void *obj, void *data, QGuestAllocator *alloc)
+static void hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QTestState *qts = global_qtest;
 
-- 
2.41.0

Re: [PATCH 03/10] tests/virtio-scsi: Clean up global variable shadowing

[Thomas Huth]

On 09/10/2023 12.02, Philippe Mathieu-Daudé wrote:
> Rename the (unused) 'allow' argument, following the pattern
> used by the other tests in this file. This fixes:
> 
>    tests/qtest/virtio-scsi-test.c:159:61: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static void hotplug(void *obj, void *data, QGuestAllocator *alloc)
>                                                                ^
>    tests/qtest/virtio-scsi-test.c:37:25: note: previous declaration is here
>    static QGuestAllocator *alloc;
>                            ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/virtio-scsi-test.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/virtio-scsi-test.c b/tests/qtest/virtio-scsi-test.c
> index ceaa7f2415..db10d572d0 100644
> --- a/tests/qtest/virtio-scsi-test.c
> +++ b/tests/qtest/virtio-scsi-test.c
> @@ -156,7 +156,7 @@ static QVirtioSCSIQueues *qvirtio_scsi_init(QVirtioDevice *dev)
>       return vs;
>   }
>   
> -static void hotplug(void *obj, void *data, QGuestAllocator *alloc)
> +static void hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
>   {
>       QTestState *qts = global_qtest;
>   

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 03/10] tests/virtio-scsi: Clean up global variable shadowing

[Manos Pitsidianakis]

On Mon, 09 Oct 2023 13:02, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>Rename the (unused) 'allow' argument, following the pattern

s/allow/alloc

Otherwise,

Reviewed-By: Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>

>used by the other tests in this file. This fixes:
>
>  tests/qtest/virtio-scsi-test.c:159:61: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>  static void hotplug(void *obj, void *data, QGuestAllocator *alloc)
>                                                              ^
>  tests/qtest/virtio-scsi-test.c:37:25: note: previous declaration is here
>  static QGuestAllocator *alloc;
>                          ^
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> tests/qtest/virtio-scsi-test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/tests/qtest/virtio-scsi-test.c b/tests/qtest/virtio-scsi-test.c
>index ceaa7f2415..db10d572d0 100644
>--- a/tests/qtest/virtio-scsi-test.c
>+++ b/tests/qtest/virtio-scsi-test.c
>@@ -156,7 +156,7 @@ static QVirtioSCSIQueues *qvirtio_scsi_init(QVirtioDevice *dev)
>     return vs;
> }
> 
>-static void hotplug(void *obj, void *data, QGuestAllocator *alloc)
>+static void hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
> {
>     QTestState *qts = global_qtest;
> 
>-- 
>2.41.0
>
>

[PATCH 04/10] tests/cdrom-test: Clean up global variable shadowing in prepare_image()

[Philippe Mathieu-Daudé]

Rename the variable to fix:

  tests/qtest/cdrom-test.c:40:50: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static int prepare_image(const char *arch, char *isoimage)
                                                   ^
  tests/qtest/cdrom-test.c:18:13: note: previous declaration is here
  static char isoimage[] = "cdrom-boot-iso-XXXXXX";
              ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/cdrom-test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
index f2a8d91929..0945383789 100644
--- a/tests/qtest/cdrom-test.c
+++ b/tests/qtest/cdrom-test.c
@@ -37,17 +37,17 @@ static int exec_xorrisofs(const char **args)
     return exit_status;
 }
 
-static int prepare_image(const char *arch, char *isoimage)
+static int prepare_image(const char *arch, char *isoimagepath)
 {
     char srcdir[] = "cdrom-test-dir-XXXXXX";
     char *codefile = NULL;
     int ifh, ret = -1;
     const char *args[] = {
         "xorrisofs", "-quiet", "-l", "-no-emul-boot",
-        "-b", NULL, "-o", isoimage, srcdir, NULL
+        "-b", NULL, "-o", isoimagepath, srcdir, NULL
     };
 
-    ifh = mkstemp(isoimage);
+    ifh = mkstemp(isoimagepath);
     if (ifh < 0) {
         perror("Error creating temporary iso image file");
         return -1;
-- 
2.41.0

Re: [PATCH 04/10] tests/cdrom-test: Clean up global variable shadowing in prepare_image()

[John Snow]

On Mon, Oct 9, 2023 at 6:03 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Rename the variable to fix:
>
>   tests/qtest/cdrom-test.c:40:50: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>   static int prepare_image(const char *arch, char *isoimage)
>                                                    ^
>   tests/qtest/cdrom-test.c:18:13: note: previous declaration is here
>   static char isoimage[] = "cdrom-boot-iso-XXXXXX";
>               ^
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  tests/qtest/cdrom-test.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
> index f2a8d91929..0945383789 100644
> --- a/tests/qtest/cdrom-test.c
> +++ b/tests/qtest/cdrom-test.c
> @@ -37,17 +37,17 @@ static int exec_xorrisofs(const char **args)
>      return exit_status;
>  }
>
> -static int prepare_image(const char *arch, char *isoimage)
> +static int prepare_image(const char *arch, char *isoimagepath)
>  {
>      char srcdir[] = "cdrom-test-dir-XXXXXX";
>      char *codefile = NULL;
>      int ifh, ret = -1;
>      const char *args[] = {
>          "xorrisofs", "-quiet", "-l", "-no-emul-boot",
> -        "-b", NULL, "-o", isoimage, srcdir, NULL
> +        "-b", NULL, "-o", isoimagepath, srcdir, NULL
>      };
>
> -    ifh = mkstemp(isoimage);
> +    ifh = mkstemp(isoimagepath);
>      if (ifh < 0) {
>          perror("Error creating temporary iso image file");
>          return -1;
> --
> 2.41.0
>

[PATCH 05/10] tests/hd-geo-test: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Rename the variable to fix:

  tests/qtest/hd-geo-test.c:610:11: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      char *img_file_name;
            ^
  tests/qtest/hd-geo-test.c:80:14: note: previous declaration is here
  static char *img_file_name[backend_last];
               ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/hd-geo-test.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index d08bffad91..e5e28c412d 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -78,6 +78,7 @@ static const CHST hd_chst[backend_last][mbr_last] = {
 };
 
 static char *img_file_name[backend_last];
+static char *qcow2_imgpath;
 
 static const CHST *cur_ide[4];
 
@@ -607,18 +608,17 @@ static TestArgs *create_args(void)
 static void add_drive_with_mbr(TestArgs *args,
                                MBRpartitions mbr, uint64_t sectors)
 {
-    char *img_file_name;
     char part[300];
     int ret;
 
     g_assert(args->n_drives < MAX_DRIVES);
 
-    img_file_name = create_qcow2_with_mbr(mbr, sectors);
+    qcow2_imgpath = create_qcow2_with_mbr(mbr, sectors);
 
-    args->drives[args->n_drives] = img_file_name;
+    args->drives[args->n_drives] = qcow2_imgpath;
     ret = snprintf(part, sizeof(part),
                    "-drive file=%s,if=none,format=qcow2,id=disk%d",
-                   img_file_name, args->n_drives);
+                   qcow2_imgpath, args->n_drives);
     g_assert((0 < ret) && (ret <= sizeof(part)));
     args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, g_strdup(part));
     args->n_drives++;
@@ -1139,6 +1139,10 @@ test_add_done:
             g_free(img_file_name[i]);
         }
     }
+    if (qcow2_imgpath) {
+        unlink(qcow2_imgpath);
+        g_free(qcow2_imgpath);
+    }
 
     return ret;
 }
-- 
2.41.0

Re: [PATCH 05/10] tests/hd-geo-test: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

On 9/10/23 12:02, Philippe Mathieu-Daudé wrote:
> Rename the variable to fix:
> 
>    tests/qtest/hd-geo-test.c:610:11: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>        char *img_file_name;
>              ^
>    tests/qtest/hd-geo-test.c:80:14: note: previous declaration is here
>    static char *img_file_name[backend_last];
>                 ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/hd-geo-test.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> index d08bffad91..e5e28c412d 100644
> --- a/tests/qtest/hd-geo-test.c
> +++ b/tests/qtest/hd-geo-test.c
> @@ -78,6 +78,7 @@ static const CHST hd_chst[backend_last][mbr_last] = {
>   };
>   
>   static char *img_file_name[backend_last];
> +static char *qcow2_imgpath;
>   
>   static const CHST *cur_ide[4];
>   
> @@ -607,18 +608,17 @@ static TestArgs *create_args(void)
>   static void add_drive_with_mbr(TestArgs *args,
>                                  MBRpartitions mbr, uint64_t sectors)
>   {
> -    char *img_file_name;
>       char part[300];
>       int ret;
>   
>       g_assert(args->n_drives < MAX_DRIVES);
>   
> -    img_file_name = create_qcow2_with_mbr(mbr, sectors);
> +    qcow2_imgpath = create_qcow2_with_mbr(mbr, sectors);
>   
> -    args->drives[args->n_drives] = img_file_name;
> +    args->drives[args->n_drives] = qcow2_imgpath;
>       ret = snprintf(part, sizeof(part),
>                      "-drive file=%s,if=none,format=qcow2,id=disk%d",
> -                   img_file_name, args->n_drives);
> +                   qcow2_imgpath, args->n_drives);
>       g_assert((0 < ret) && (ret <= sizeof(part)));
>       args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, g_strdup(part));
>       args->n_drives++;
> @@ -1139,6 +1139,10 @@ test_add_done:
>               g_free(img_file_name[i]);
>           }
>       }
> +    if (qcow2_imgpath) {
> +        unlink(qcow2_imgpath);
> +        g_free(qcow2_imgpath);

Oops I squashed this part while rebasing, this was supposed to
be a different patch.

> +    }
>   
>       return ret;
>   }

[PATCH 06/10] tests/rtl8139: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Rename the variable to fix:

  tests/qtest/rtl8139-test.c:28:33: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void save_fn(QPCIDevice *dev, int devfn, void *data)
                                  ^
  tests/qtest/rtl8139-test.c:37:17: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      QPCIDevice *dev;
                  ^
  tests/qtest/rtl8139-test.c:25:20: note: previous declaration is here
  static QPCIDevice *dev;
                     ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/rtl8139-test.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/rtl8139-test.c b/tests/qtest/rtl8139-test.c
index 4dc0a0d22e..eedf90f65a 100644
--- a/tests/qtest/rtl8139-test.c
+++ b/tests/qtest/rtl8139-test.c
@@ -22,7 +22,7 @@ static void nop(void)
 #define CLK 33333333
 
 static QPCIBus *pcibus;
-static QPCIDevice *dev;
+static QPCIDevice *pcidev;
 static QPCIBar dev_bar;
 
 static void save_fn(QPCIDevice *dev, int devfn, void *data)
@@ -46,7 +46,7 @@ static QPCIDevice *get_device(void)
 #define PORT(name, len, val) \
 static unsigned __attribute__((unused)) in_##name(void) \
 { \
-    unsigned res = qpci_io_read##len(dev, dev_bar, (val));     \
+    unsigned res = qpci_io_read##len(pcidev, dev_bar, (val));     \
     if (verbosity_level >= 2) { \
         g_test_message("*%s -> %x", #name, res); \
     } \
@@ -57,7 +57,7 @@ static void out_##name(unsigned v) \
     if (verbosity_level >= 2) { \
         g_test_message("%x -> *%s", v, #name); \
     } \
-    qpci_io_write##len(dev, dev_bar, (val), v);        \
+    qpci_io_write##len(pcidev, dev_bar, (val), v);        \
 }
 
 PORT(Timer, l, 0x48)
@@ -189,11 +189,11 @@ static void test_init(void)
 {
     uint64_t barsize;
 
-    dev = get_device();
+    pcidev = get_device();
 
-    dev_bar = qpci_iomap(dev, 0, &barsize);
+    dev_bar = qpci_iomap(pcidev, 0, &barsize);
 
-    qpci_device_enable(dev);
+    qpci_device_enable(pcidev);
 
     test_timer();
 }
-- 
2.41.0

Re: [PATCH 06/10] tests/rtl8139: Clean up global variable shadowing

[Thomas Huth]

On 09/10/2023 12.02, Philippe Mathieu-Daudé wrote:
> Rename the variable to fix:
> 
>    tests/qtest/rtl8139-test.c:28:33: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static void save_fn(QPCIDevice *dev, int devfn, void *data)
>                                    ^
>    tests/qtest/rtl8139-test.c:37:17: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>        QPCIDevice *dev;
>                    ^
>    tests/qtest/rtl8139-test.c:25:20: note: previous declaration is here
>    static QPCIDevice *dev;
>                       ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/rtl8139-test.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 07/10] tests/npcm7xx_adc: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Rename the global 'adc' variable in order to avoid:

  tests/qtest/npcm7xx_adc-test.c:98:58: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static uint32_t adc_read_con(QTestState *qts, const ADC *adc)
                                                           ^
  tests/qtest/npcm7xx_adc-test.c:103:55: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void adc_write_con(QTestState *qts, const ADC *adc, uint32_t value)
                                                        ^
  tests/qtest/npcm7xx_adc-test.c:108:59: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static uint32_t adc_read_data(QTestState *qts, const ADC *adc)
                                                            ^
  tests/qtest/npcm7xx_adc-test.c:119:53: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void adc_qom_set(QTestState *qts, const ADC *adc,
                                                      ^
  tests/qtest/npcm7xx_adc-test.c:135:57: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void adc_write_input(QTestState *qts, const ADC *adc,
                                                          ^
  tests/qtest/npcm7xx_adc-test.c:144:56: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void adc_write_vref(QTestState *qts, const ADC *adc, uint32_t value)
                                                         ^
  tests/qtest/npcm7xx_adc-test.c:162:59: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static uint32_t adc_prescaler(QTestState *qts, const ADC *adc)
                                                            ^
  tests/qtest/npcm7xx_adc-test.c:175:64: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void adc_wait_conv_finished(QTestState *qts, const ADC *adc,
                                                                 ^
  tests/qtest/npcm7xx_adc-test.c:196:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
    const ADC *adc = adc_p;
               ^
  tests/qtest/npcm7xx_adc-test.c:207:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
    const ADC *adc = adc_p;
               ^
  tests/qtest/npcm7xx_adc-test.c:235:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
    const ADC *adc = adc_p;
               ^
  tests/qtest/npcm7xx_adc-test.c:267:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
    const ADC *adc = adc_p;
               ^
  tests/qtest/npcm7xx_adc-test.c:293:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
    const ADC *adc = adc_p;
               ^
  tests/qtest/npcm7xx_adc-test.c:311:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
    const ADC *adc = adc_p;
               ^
  tests/qtest/npcm7xx_adc-test.c:93:5: note: previous declaration is here
  ADC adc = {
      ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/npcm7xx_adc-test.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/npcm7xx_adc-test.c b/tests/qtest/npcm7xx_adc-test.c
index 8048044d28..e751a72e36 100644
--- a/tests/qtest/npcm7xx_adc-test.c
+++ b/tests/qtest/npcm7xx_adc-test.c
@@ -90,7 +90,7 @@ typedef struct ADC {
     uint64_t base_addr;
 } ADC;
 
-ADC adc = {
+ADC adc_defs = {
     .irq        = 0,
     .base_addr  = 0xf000c000
 };
@@ -367,12 +367,12 @@ int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
 
-    add_test(init, &adc);
-    add_test(convert_internal, &adc);
-    add_test(convert_external, &adc);
-    add_test(interrupt, &adc);
-    add_test(reset, &adc);
-    add_test(calibrate, &adc);
+    add_test(init, &adc_defs);
+    add_test(convert_internal, &adc_defs);
+    add_test(convert_external, &adc_defs);
+    add_test(interrupt, &adc_defs);
+    add_test(reset, &adc_defs);
+    add_test(calibrate, &adc_defs);
 
     return g_test_run();
 }
-- 
2.41.0

Re: [PATCH 07/10] tests/npcm7xx_adc: Clean up global variable shadowing

[Thomas Huth]

On 09/10/2023 12.02, Philippe Mathieu-Daudé wrote:
> Rename the global 'adc' variable in order to avoid:
> 
>    tests/qtest/npcm7xx_adc-test.c:98:58: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static uint32_t adc_read_con(QTestState *qts, const ADC *adc)
>                                                             ^
>    tests/qtest/npcm7xx_adc-test.c:103:55: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static void adc_write_con(QTestState *qts, const ADC *adc, uint32_t value)
>                                                          ^
>    tests/qtest/npcm7xx_adc-test.c:108:59: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static uint32_t adc_read_data(QTestState *qts, const ADC *adc)
>                                                              ^
>    tests/qtest/npcm7xx_adc-test.c:119:53: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static void adc_qom_set(QTestState *qts, const ADC *adc,
>                                                        ^
>    tests/qtest/npcm7xx_adc-test.c:135:57: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static void adc_write_input(QTestState *qts, const ADC *adc,
>                                                            ^
>    tests/qtest/npcm7xx_adc-test.c:144:56: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static void adc_write_vref(QTestState *qts, const ADC *adc, uint32_t value)
>                                                           ^
>    tests/qtest/npcm7xx_adc-test.c:162:59: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static uint32_t adc_prescaler(QTestState *qts, const ADC *adc)
>                                                              ^
>    tests/qtest/npcm7xx_adc-test.c:175:64: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static void adc_wait_conv_finished(QTestState *qts, const ADC *adc,
>                                                                   ^
>    tests/qtest/npcm7xx_adc-test.c:196:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>      const ADC *adc = adc_p;
>                 ^
>    tests/qtest/npcm7xx_adc-test.c:207:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>      const ADC *adc = adc_p;
>                 ^
>    tests/qtest/npcm7xx_adc-test.c:235:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>      const ADC *adc = adc_p;
>                 ^
>    tests/qtest/npcm7xx_adc-test.c:267:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>      const ADC *adc = adc_p;
>                 ^
>    tests/qtest/npcm7xx_adc-test.c:293:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>      const ADC *adc = adc_p;
>                 ^
>    tests/qtest/npcm7xx_adc-test.c:311:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>      const ADC *adc = adc_p;
>                 ^
>    tests/qtest/npcm7xx_adc-test.c:93:5: note: previous declaration is here
>    ADC adc = {
>        ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/npcm7xx_adc-test.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 08/10] tests/aio: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Rename the argument to fix:

  tests/unit/test-aio.c:130:44: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void set_event_notifier(AioContext *ctx, EventNotifier *notifier,
                                             ^
  tests/unit/test-aio.c:22:20: note: previous declaration is here
  static AioContext *ctx;
                     ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/unit/test-aio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/unit/test-aio.c b/tests/unit/test-aio.c
index 71ed31a4db..337b6e4ea7 100644
--- a/tests/unit/test-aio.c
+++ b/tests/unit/test-aio.c
@@ -127,10 +127,10 @@ static void *test_acquire_thread(void *opaque)
     return NULL;
 }
 
-static void set_event_notifier(AioContext *ctx, EventNotifier *notifier,
+static void set_event_notifier(AioContext *nctx, EventNotifier *notifier,
                                EventNotifierHandler *handler)
 {
-    aio_set_event_notifier(ctx, notifier, handler, NULL, NULL);
+    aio_set_event_notifier(nctx, notifier, handler, NULL, NULL);
 }
 
 static void dummy_notifier_read(EventNotifier *n)
-- 
2.41.0

Re: [PATCH 08/10] tests/aio: Clean up global variable shadowing

[Thomas Huth]

On 09/10/2023 12.02, Philippe Mathieu-Daudé wrote:
> Rename the argument to fix:
> 
>    tests/unit/test-aio.c:130:44: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static void set_event_notifier(AioContext *ctx, EventNotifier *notifier,
>                                               ^
>    tests/unit/test-aio.c:22:20: note: previous declaration is here
>    static AioContext *ctx;
>                       ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/unit/test-aio.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/unit/test-aio.c b/tests/unit/test-aio.c
> index 71ed31a4db..337b6e4ea7 100644
> --- a/tests/unit/test-aio.c
> +++ b/tests/unit/test-aio.c
> @@ -127,10 +127,10 @@ static void *test_acquire_thread(void *opaque)
>       return NULL;
>   }
>   
> -static void set_event_notifier(AioContext *ctx, EventNotifier *notifier,
> +static void set_event_notifier(AioContext *nctx, EventNotifier *notifier,
>                                  EventNotifierHandler *handler)
>   {
> -    aio_set_event_notifier(ctx, notifier, handler, NULL, NULL);
> +    aio_set_event_notifier(nctx, notifier, handler, NULL, NULL);
>   }
>   
>   static void dummy_notifier_read(EventNotifier *n)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 09/10] tests/aio-multithread: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Rename the argument to avoid:

  tests/unit/test-aio-multithread.c:226:37: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void test_multi_co_mutex(int threads, int seconds)
                                      ^
  tests/unit/test-aio-multithread.c:401:34: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void test_multi_mutex(int threads, int seconds)
                                   ^
  tests/unit/test-aio-multithread.c:24:18: note: previous declaration is here
  static IOThread *threads[NUM_CONTEXTS];
                   ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/unit/test-aio-multithread.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/unit/test-aio-multithread.c b/tests/unit/test-aio-multithread.c
index 08d4570ccb..d587f20667 100644
--- a/tests/unit/test-aio-multithread.c
+++ b/tests/unit/test-aio-multithread.c
@@ -223,7 +223,7 @@ static void coroutine_fn test_multi_co_mutex_entry(void *opaque)
     qatomic_dec(&running);
 }
 
-static void test_multi_co_mutex(int threads, int seconds)
+static void test_multi_co_mutex(unsigned ctx_num, int seconds)
 {
     int i;
 
@@ -233,9 +233,9 @@ static void test_multi_co_mutex(int threads, int seconds)
     now_stopping = false;
 
     create_aio_contexts();
-    assert(threads <= NUM_CONTEXTS);
-    running = threads;
-    for (i = 0; i < threads; i++) {
+    assert(ctx_num <= NUM_CONTEXTS);
+    running = ctx_num;
+    for (i = 0; i < ctx_num; i++) {
         Coroutine *co1 = qemu_coroutine_create(test_multi_co_mutex_entry, NULL);
         aio_co_schedule(ctx[i], co1);
     }
@@ -398,7 +398,7 @@ static void test_multi_mutex_entry(void *opaque)
     qatomic_dec(&running);
 }
 
-static void test_multi_mutex(int threads, int seconds)
+static void test_multi_mutex(unsigned ctx_num, int seconds)
 {
     int i;
 
@@ -408,9 +408,9 @@ static void test_multi_mutex(int threads, int seconds)
     now_stopping = false;
 
     create_aio_contexts();
-    assert(threads <= NUM_CONTEXTS);
-    running = threads;
-    for (i = 0; i < threads; i++) {
+    assert(ctx_num <= NUM_CONTEXTS);
+    running = ctx_num;
+    for (i = 0; i < ctx_num; i++) {
         Coroutine *co1 = qemu_coroutine_create(test_multi_mutex_entry, NULL);
         aio_co_schedule(ctx[i], co1);
     }
-- 
2.41.0

Re: [PATCH 09/10] tests/aio-multithread: Clean up global variable shadowing

[Thomas Huth]

On 09/10/2023 12.02, Philippe Mathieu-Daudé wrote:
> Rename the argument to avoid:
> 
>    tests/unit/test-aio-multithread.c:226:37: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static void test_multi_co_mutex(int threads, int seconds)
>                                        ^
>    tests/unit/test-aio-multithread.c:401:34: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    static void test_multi_mutex(int threads, int seconds)
>                                     ^
>    tests/unit/test-aio-multithread.c:24:18: note: previous declaration is here
>    static IOThread *threads[NUM_CONTEXTS];
>                     ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/unit/test-aio-multithread.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/unit/test-aio-multithread.c b/tests/unit/test-aio-multithread.c
> index 08d4570ccb..d587f20667 100644
> --- a/tests/unit/test-aio-multithread.c
> +++ b/tests/unit/test-aio-multithread.c
> @@ -223,7 +223,7 @@ static void coroutine_fn test_multi_co_mutex_entry(void *opaque)
>       qatomic_dec(&running);
>   }
>   
> -static void test_multi_co_mutex(int threads, int seconds)
> +static void test_multi_co_mutex(unsigned ctx_num, int seconds)

Why did you change the type of the variable? ... you should at least mention 
this in the commit description.

>   {
>       int i;
>   
> @@ -233,9 +233,9 @@ static void test_multi_co_mutex(int threads, int seconds)
>       now_stopping = false;
>   
>       create_aio_contexts();
> -    assert(threads <= NUM_CONTEXTS);
> -    running = threads;
> -    for (i = 0; i < threads; i++) {
> +    assert(ctx_num <= NUM_CONTEXTS);
> +    running = ctx_num;
> +    for (i = 0; i < ctx_num; i++) {
>           Coroutine *co1 = qemu_coroutine_create(test_multi_co_mutex_entry, NULL);
>           aio_co_schedule(ctx[i], co1);
>       }
> @@ -398,7 +398,7 @@ static void test_multi_mutex_entry(void *opaque)
>       qatomic_dec(&running);
>   }
>   
> -static void test_multi_mutex(int threads, int seconds)
> +static void test_multi_mutex(unsigned ctx_num, int seconds)

dito.

  Thomas

>   {
>       int i;
>   
> @@ -408,9 +408,9 @@ static void test_multi_mutex(int threads, int seconds)
>       now_stopping = false;
>   
>       create_aio_contexts();
> -    assert(threads <= NUM_CONTEXTS);
> -    running = threads;
> -    for (i = 0; i < threads; i++) {
> +    assert(ctx_num <= NUM_CONTEXTS);
> +    running = ctx_num;
> +    for (i = 0; i < ctx_num; i++) {
>           Coroutine *co1 = qemu_coroutine_create(test_multi_mutex_entry, NULL);
>           aio_co_schedule(ctx[i], co1);
>       }

[PATCH 10/10] tests/coroutine: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Rename the global variable to avoid:

  tests/unit/test-coroutine.c:430:11: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      bool *done = opaque;
            ^
  tests/unit/test-coroutine.c:438:10: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      bool done = false;
           ^
  tests/unit/test-coroutine.c:198:12: note: previous declaration is here
  static int done;
             ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/unit/test-coroutine.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/unit/test-coroutine.c b/tests/unit/test-coroutine.c
index b0d21d673a..29695cbdcb 100644
--- a/tests/unit/test-coroutine.c
+++ b/tests/unit/test-coroutine.c
@@ -195,7 +195,7 @@ static void test_no_dangling_access(void)
 }
 
 static bool locked;
-static int done;
+static int done_count;
 
 static void coroutine_fn mutex_fn(void *opaque)
 {
@@ -206,7 +206,7 @@ static void coroutine_fn mutex_fn(void *opaque)
     qemu_coroutine_yield();
     locked = false;
     qemu_co_mutex_unlock(m);
-    done++;
+    done_count++;
 }
 
 static void coroutine_fn lockable_fn(void *opaque)
@@ -218,7 +218,7 @@ static void coroutine_fn lockable_fn(void *opaque)
     qemu_coroutine_yield();
     locked = false;
     qemu_lockable_unlock(x);
-    done++;
+    done_count++;
 }
 
 static void do_test_co_mutex(CoroutineEntry *entry, void *opaque)
@@ -226,7 +226,7 @@ static void do_test_co_mutex(CoroutineEntry *entry, void *opaque)
     Coroutine *c1 = qemu_coroutine_create(entry, opaque);
     Coroutine *c2 = qemu_coroutine_create(entry, opaque);
 
-    done = 0;
+    done_count = 0;
     qemu_coroutine_enter(c1);
     g_assert(locked);
     qemu_coroutine_enter(c2);
@@ -235,11 +235,11 @@ static void do_test_co_mutex(CoroutineEntry *entry, void *opaque)
      * terminates.
      */
     qemu_coroutine_enter(c1);
-    g_assert_cmpint(done, ==, 1);
+    g_assert_cmpint(done_count, ==, 1);
     g_assert(locked);
 
     qemu_coroutine_enter(c2);
-    g_assert_cmpint(done, ==, 2);
+    g_assert_cmpint(done_count, ==, 2);
     g_assert(!locked);
 }
 
-- 
2.41.0

Re: [PATCH 10/10] tests/coroutine: Clean up global variable shadowing

[Thomas Huth]

On 09/10/2023 12.02, Philippe Mathieu-Daudé wrote:
> Rename the global variable to avoid:
> 
>    tests/unit/test-coroutine.c:430:11: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>        bool *done = opaque;
>              ^
>    tests/unit/test-coroutine.c:438:10: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>        bool done = false;
>             ^
>    tests/unit/test-coroutine.c:198:12: note: previous declaration is here
>    static int done;
>               ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/unit/test-coroutine.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 00/10] tests: Clean up global variables shadowing

[Thomas Huth]

On 09/10/2023 12.02, Philippe Mathieu-Daudé wrote:
> Clean up global variables shadowing in tests/
> in order to be able to use -Wshadow with Clang.
> 
> Philippe Mathieu-Daudé (10):
>    system/qtest: Clean up global variable shadowing in
>      qtest_server_init()
>    tests/throttle: Clean up global variable shadowing
>    tests/virtio-scsi: Clean up global variable shadowing
>    tests/cdrom-test: Clean up global variable shadowing in
>      prepare_image()
>    tests/hd-geo-test: Clean up global variable shadowing
>    tests/rtl8139: Clean up global variable shadowing
>    tests/npcm7xx_adc: Clean up global variable shadowing
>    tests/aio: Clean up global variable shadowing
>    tests/aio-multithread: Clean up global variable shadowing
>    tests/coroutine: Clean up global variable shadowing

Thanks, I've queued now everything except for patch 5 and 9 that both still 
had unresolved comments.

  Thomas