[PATCH v3 00/16] (few more) Steps towards enabling -Wshadow

[PATCH v3 00/16] (few more) Steps towards enabling -Wshadow

[Philippe Mathieu-Daudé]

Since v2:
- Rebased (many patches merged by Markus)
- Updated arm/ide patches (Markus, Peter)
- More patches for optarg (3)

Since v1:
- Addressed review comments
- Added R-b tags
- More patches

For rational see Markus cover on
https://lore.kernel.org/qemu-devel/20230831132546.3525721-1-armbru@redhat.com/

Philippe Mathieu-Daudé (16):
  hw/audio/soundhw: Clean up global variable shadowing
  hw/ide/ahci: Clean up local variable shadowing
  net/net: Clean up global variable shadowing
  os-posix: Clean up global variable shadowing
  plugins/loader: Clean up global variable shadowing
  qemu-img: Clean up global variable shadowing
  qemu-io: Clean up global variable shadowing
  qom/object_interfaces: Clean up global variable shadowing
  semihosting: Clean up global variable shadowing
  ui/cocoa: Clean up global variable shadowing
  util/cutils: Clean up global variable shadowing in
    get_relocated_path()
  util/guest-random: Clean up global variable shadowing
  semihosting/arm-compat: Clean up local variable shadowing
  softmmu/vl: Clean up global variable shadowing
  sysemu/tpm: Clean up global variable shadowing
  trace/control: Clean up global variable shadowing

 include/hw/audio/soundhw.h      |  2 +-
 include/net/net.h               |  6 +++---
 include/qemu/guest-random.h     |  8 ++++----
 include/qemu/plugin.h           |  4 ++--
 include/qom/object_interfaces.h | 16 ++++++++--------
 include/semihosting/semihost.h  |  2 +-
 include/sysemu/os-posix.h       |  4 ++--
 include/sysemu/tpm.h            |  2 +-
 trace/control.h                 |  4 ++--
 hw/audio/soundhw.c              |  6 +++---
 hw/ide/ahci.c                   |  4 +---
 net/net.c                       | 14 +++++++-------
 os-posix.c                      | 12 ++++++------
 plugins/loader.c                |  4 ++--
 qemu-img.c                      | 22 +++++++++++-----------
 qemu-io.c                       |  4 ++--
 qom/object_interfaces.c         | 16 ++++++++--------
 semihosting/arm-compat-semi.c   |  5 ++++-
 semihosting/config.c            |  8 ++++----
 softmmu/tpm.c                   |  6 +++---
 softmmu/vl.c                    | 26 +++++++++++++-------------
 stubs/semihost.c                |  2 +-
 trace/control.c                 |  4 ++--
 util/cutils.c                   |  1 -
 util/guest-random.c             |  6 +++---
 ui/cocoa.m                      |  4 ++--
 26 files changed, 96 insertions(+), 96 deletions(-)

-- 
2.41.0

[PATCH v3 01/16] hw/audio/soundhw: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  hw/audio/soundhw.c:86:33: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  void select_soundhw(const char *optarg, const char *audiodev)
                                  ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
  extern char *optarg;                    /* getopt(3) external variables */
               ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/audio/soundhw.h | 2 +-
 hw/audio/soundhw.c         | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
index 270717a06a..474c5ff94e 100644
--- a/include/hw/audio/soundhw.h
+++ b/include/hw/audio/soundhw.h
@@ -8,6 +8,6 @@ void deprecated_register_soundhw(const char *name, const char *descr,
 
 void soundhw_init(void);
 void show_valid_soundhw(void);
-void select_soundhw(const char *optarg, const char *audiodev);
+void select_soundhw(const char *name, const char *audiodev);
 
 #endif
diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
index 94d9463e42..b387b0ef7d 100644
--- a/hw/audio/soundhw.c
+++ b/hw/audio/soundhw.c
@@ -83,7 +83,7 @@ void show_valid_soundhw(void)
 static struct soundhw *selected = NULL;
 static const char *audiodev_id;
 
-void select_soundhw(const char *optarg, const char *audiodev)
+void select_soundhw(const char *name, const char *audiodev)
 {
     struct soundhw *c;
 
@@ -92,7 +92,7 @@ void select_soundhw(const char *optarg, const char *audiodev)
     }
 
     for (c = soundhw; c->name; ++c) {
-        if (g_str_equal(c->name, optarg)) {
+        if (g_str_equal(c->name, name)) {
             selected = c;
             audiodev_id = audiodev;
             break;
@@ -100,7 +100,7 @@ void select_soundhw(const char *optarg, const char *audiodev)
     }
 
     if (!c->name) {
-        error_report("Unknown sound card name `%s'", optarg);
+        error_report("Unknown sound card name `%s'", name);
         show_valid_soundhw();
         exit(1);
     }
-- 
2.41.0

Re: [PATCH v3 01/16] hw/audio/soundhw: Clean up global variable shadowing

[Thomas Huth]

On 04/10/2023 14.00, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>    hw/audio/soundhw.c:86:33: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    void select_soundhw(const char *optarg, const char *audiodev)
>                                    ^
>    /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>    extern char *optarg;                    /* getopt(3) external variables */
>                 ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/audio/soundhw.h | 2 +-
>   hw/audio/soundhw.c         | 6 +++---
>   2 files changed, 4 insertions(+), 4 deletions(-)

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

[PATCH v3 02/16] hw/ide/ahci: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  hw/ide/ahci.c:1577:23: error: declaration shadows a local variable [-Werror,-Wshadow]
            IDEState *s = &ad->port.ifs[j];
                      ^
  hw/ide/ahci.c:1569:29: note: previous declaration is here
    void ahci_uninit(AHCIState *s)
                                ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/ahci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index d0a774bc17..fcc5476e9e 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1622,9 +1622,7 @@ void ahci_uninit(AHCIState *s)
         AHCIDevice *ad = &s->dev[i];
 
         for (j = 0; j < 2; j++) {
-            IDEState *s = &ad->port.ifs[j];
-
-            ide_exit(s);
+            ide_exit(&ad->port.ifs[j]);
         }
         object_unparent(OBJECT(&ad->port));
     }
-- 
2.41.0

Re: [PATCH v3 02/16] hw/ide/ahci: Clean up local variable shadowing

[John Snow]

On Wed, Oct 4, 2023 at 8:00 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Fix:
>
>   hw/ide/ahci.c:1577:23: error: declaration shadows a local variable [-Werror,-Wshadow]
>             IDEState *s = &ad->port.ifs[j];
>                       ^
>   hw/ide/ahci.c:1569:29: note: previous declaration is here
>     void ahci_uninit(AHCIState *s)
>                                 ^
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

> ---
>  hw/ide/ahci.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index d0a774bc17..fcc5476e9e 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1622,9 +1622,7 @@ void ahci_uninit(AHCIState *s)
>          AHCIDevice *ad = &s->dev[i];
>
>          for (j = 0; j < 2; j++) {
> -            IDEState *s = &ad->port.ifs[j];
> -
> -            ide_exit(s);
> +            ide_exit(&ad->port.ifs[j]);
>          }
>          object_unparent(OBJECT(&ad->port));
>      }
> --
> 2.41.0
>

[PATCH v3 03/16] net/net: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  net/net.c:1680:35: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  bool netdev_is_modern(const char *optarg)
                                    ^
  net/net.c:1714:38: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  void netdev_parse_modern(const char *optarg)
                                       ^
  net/net.c:1728:60: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  void net_client_parse(QemuOptsList *opts_list, const char *optarg)
                                                             ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
  extern char *optarg;                    /* getopt(3) external variables */
               ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/net/net.h |  6 +++---
 net/net.c         | 14 +++++++-------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 330d285930..2fb1c9181c 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -247,9 +247,9 @@ extern const char *host_net_devices[];
 
 /* from net.c */
 extern NetClientStateList net_clients;
-bool netdev_is_modern(const char *optarg);
-void netdev_parse_modern(const char *optarg);
-void net_client_parse(QemuOptsList *opts_list, const char *str);
+bool netdev_is_modern(const char *optstr);
+void netdev_parse_modern(const char *optstr);
+void net_client_parse(QemuOptsList *opts_list, const char *optstr);
 void show_netdevs(void);
 void net_init_clients(void);
 void net_check_clients(void);
diff --git a/net/net.c b/net/net.c
index 1c0bfdaa6c..c0c0cbe99e 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1677,7 +1677,7 @@ void net_init_clients(void)
  * Modern syntax is to be parsed with netdev_parse_modern().
  * Traditional syntax is to be parsed with net_client_parse().
  */
-bool netdev_is_modern(const char *optarg)
+bool netdev_is_modern(const char *optstr)
 {
     QemuOpts *opts;
     bool is_modern;
@@ -1689,13 +1689,13 @@ bool netdev_is_modern(const char *optarg)
         .desc = { { } },
     };
 
-    if (optarg[0] == '{') {
+    if (optstr[0] == '{') {
         /* This is JSON, which means it's modern syntax */
         return true;
     }
 
     opts = qemu_opts_create(&dummy_opts, NULL, false, &error_abort);
-    qemu_opts_do_parse(opts, optarg, dummy_opts.implied_opt_name,
+    qemu_opts_do_parse(opts, optstr, dummy_opts.implied_opt_name,
                        &error_abort);
     type = qemu_opt_get(opts, "type");
     is_modern = !g_strcmp0(type, "stream") || !g_strcmp0(type, "dgram");
@@ -1711,12 +1711,12 @@ bool netdev_is_modern(const char *optarg)
  * netdev_parse_modern() appends to @nd_queue, whereas net_client_parse()
  * appends to @qemu_netdev_opts.
  */
-void netdev_parse_modern(const char *optarg)
+void netdev_parse_modern(const char *optstr)
 {
     Visitor *v;
     NetdevQueueEntry *nd;
 
-    v = qobject_input_visitor_new_str(optarg, "type", &error_fatal);
+    v = qobject_input_visitor_new_str(optstr, "type", &error_fatal);
     nd = g_new(NetdevQueueEntry, 1);
     visit_type_Netdev(v, NULL, &nd->nd, &error_fatal);
     visit_free(v);
@@ -1725,9 +1725,9 @@ void netdev_parse_modern(const char *optarg)
     QSIMPLEQ_INSERT_TAIL(&nd_queue, nd, entry);
 }
 
-void net_client_parse(QemuOptsList *opts_list, const char *optarg)
+void net_client_parse(QemuOptsList *opts_list, const char *optstr)
 {
-    if (!qemu_opts_parse_noisily(opts_list, optarg, true)) {
+    if (!qemu_opts_parse_noisily(opts_list, optstr, true)) {
         exit(1);
     }
 }
-- 
2.41.0

Re: [PATCH v3 03/16] net/net: Clean up global variable shadowing

[Thomas Huth]

On 04/10/2023 14.00, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>    net/net.c:1680:35: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    bool netdev_is_modern(const char *optarg)
>                                      ^
>    net/net.c:1714:38: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    void netdev_parse_modern(const char *optarg)
>                                         ^
>    net/net.c:1728:60: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    void net_client_parse(QemuOptsList *opts_list, const char *optarg)
>                                                               ^
>    /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>    extern char *optarg;                    /* getopt(3) external variables */
>                 ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/net/net.h |  6 +++---
>   net/net.c         | 14 +++++++-------
>   2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/include/net/net.h b/include/net/net.h
> index 330d285930..2fb1c9181c 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -247,9 +247,9 @@ extern const char *host_net_devices[];
>   
>   /* from net.c */
>   extern NetClientStateList net_clients;
> -bool netdev_is_modern(const char *optarg);
> -void netdev_parse_modern(const char *optarg);
> -void net_client_parse(QemuOptsList *opts_list, const char *str);
> +bool netdev_is_modern(const char *optstr);
> +void netdev_parse_modern(const char *optstr);
> +void net_client_parse(QemuOptsList *opts_list, const char *optstr);
>   void show_netdevs(void);
>   void net_init_clients(void);
>   void net_check_clients(void);
> diff --git a/net/net.c b/net/net.c
> index 1c0bfdaa6c..c0c0cbe99e 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1677,7 +1677,7 @@ void net_init_clients(void)
>    * Modern syntax is to be parsed with netdev_parse_modern().
>    * Traditional syntax is to be parsed with net_client_parse().
>    */
> -bool netdev_is_modern(const char *optarg)
> +bool netdev_is_modern(const char *optstr)
>   {
>       QemuOpts *opts;
>       bool is_modern;
> @@ -1689,13 +1689,13 @@ bool netdev_is_modern(const char *optarg)
>           .desc = { { } },
>       };
>   
> -    if (optarg[0] == '{') {
> +    if (optstr[0] == '{') {
>           /* This is JSON, which means it's modern syntax */
>           return true;
>       }
>   
>       opts = qemu_opts_create(&dummy_opts, NULL, false, &error_abort);
> -    qemu_opts_do_parse(opts, optarg, dummy_opts.implied_opt_name,
> +    qemu_opts_do_parse(opts, optstr, dummy_opts.implied_opt_name,
>                          &error_abort);
>       type = qemu_opt_get(opts, "type");
>       is_modern = !g_strcmp0(type, "stream") || !g_strcmp0(type, "dgram");
> @@ -1711,12 +1711,12 @@ bool netdev_is_modern(const char *optarg)
>    * netdev_parse_modern() appends to @nd_queue, whereas net_client_parse()
>    * appends to @qemu_netdev_opts.
>    */
> -void netdev_parse_modern(const char *optarg)
> +void netdev_parse_modern(const char *optstr)
>   {
>       Visitor *v;
>       NetdevQueueEntry *nd;
>   
> -    v = qobject_input_visitor_new_str(optarg, "type", &error_fatal);
> +    v = qobject_input_visitor_new_str(optstr, "type", &error_fatal);
>       nd = g_new(NetdevQueueEntry, 1);
>       visit_type_Netdev(v, NULL, &nd->nd, &error_fatal);
>       visit_free(v);
> @@ -1725,9 +1725,9 @@ void netdev_parse_modern(const char *optarg)
>       QSIMPLEQ_INSERT_TAIL(&nd_queue, nd, entry);
>   }
>   
> -void net_client_parse(QemuOptsList *opts_list, const char *optarg)
> +void net_client_parse(QemuOptsList *opts_list, const char *optstr)
>   {
> -    if (!qemu_opts_parse_noisily(opts_list, optarg, true)) {
> +    if (!qemu_opts_parse_noisily(opts_list, optstr, true)) {
>           exit(1);
>       }
>   }

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

[PATCH v3 04/16] os-posix: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  os-posix.c:103:31: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  bool os_set_runas(const char *optarg)
                                ^
  os-posix.c:176:32: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  void os_set_chroot(const char *optarg)
                                 ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
  extern char *optarg;                    /* getopt(3) external variables */
               ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/sysemu/os-posix.h |  4 ++--
 os-posix.c                | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 6dfdcbb086..dff32ae185 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -49,8 +49,8 @@ void os_setup_signal_handling(void);
 int os_set_daemonize(bool d);
 bool is_daemonized(void);
 void os_daemonize(void);
-bool os_set_runas(const char *optarg);
-void os_set_chroot(const char *optarg);
+bool os_set_runas(const char *user_id);
+void os_set_chroot(const char *path);
 void os_setup_post(void);
 int os_mlock(void);
 
diff --git a/os-posix.c b/os-posix.c
index f90dfda9b0..52ef6990ff 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -94,13 +94,13 @@ static uid_t user_uid = (uid_t)-1; /*   -1      -1        >=0    */
 static gid_t user_gid = (gid_t)-1; /*   -1      -1        >=0    */
 
 /*
- * Prepare to change user ID. optarg can be one of 3 forms:
+ * Prepare to change user ID. user_id can be one of 3 forms:
  *   - a username, in which case user ID will be changed to its uid,
  *     with primary and supplementary groups set up too;
  *   - a numeric uid, in which case only the uid will be set;
  *   - a pair of numeric uid:gid.
  */
-bool os_set_runas(const char *optarg)
+bool os_set_runas(const char *user_id)
 {
     unsigned long lv;
     const char *ep;
@@ -108,14 +108,14 @@ bool os_set_runas(const char *optarg)
     gid_t got_gid;
     int rc;
 
-    user_pwd = getpwnam(optarg);
+    user_pwd = getpwnam(user_id);
     if (user_pwd) {
         user_uid = -1;
         user_gid = -1;
         return true;
     }
 
-    rc = qemu_strtoul(optarg, &ep, 0, &lv);
+    rc = qemu_strtoul(user_id, &ep, 0, &lv);
     got_uid = lv; /* overflow here is ID in C99 */
     if (rc || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) {
         return false;
@@ -173,9 +173,9 @@ static void change_process_uid(void)
 
 static const char *chroot_dir;
 
-void os_set_chroot(const char *optarg)
+void os_set_chroot(const char *path)
 {
-    chroot_dir = optarg;
+    chroot_dir = path;
 }
 
 static void change_root(void)
-- 
2.41.0

Re: [PATCH v3 04/16] os-posix: Clean up global variable shadowing

[Eric Blake]

On Wed, Oct 04, 2023 at 02:00:07PM +0200, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>   os-posix.c:103:31: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>   bool os_set_runas(const char *optarg)
>                                 ^
>   os-posix.c:176:32: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>   void os_set_chroot(const char *optarg)
>                                  ^
>   /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>   extern char *optarg;                    /* getopt(3) external variables */
>                ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/sysemu/os-posix.h |  4 ++--
>  os-posix.c                | 12 ++++++------
>  2 files changed, 8 insertions(+), 8 deletions(-)
>

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

[PATCH v3 05/16] plugins/loader: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  include/qemu/plugin.h:245:54: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static inline void qemu_plugin_opt_parse(const char *optarg,
                                                       ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
  extern char *optarg;                    /* getopt(3) external variables */
               ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qemu/plugin.h | 4 ++--
 plugins/loader.c      | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index bc0781cab8..7fdc3a4849 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -50,7 +50,7 @@ static inline void qemu_plugin_add_opts(void)
     qemu_add_opts(&qemu_plugin_opts);
 }
 
-void qemu_plugin_opt_parse(const char *optarg, QemuPluginList *head);
+void qemu_plugin_opt_parse(const char *optstr, QemuPluginList *head);
 int qemu_plugin_load_list(QemuPluginList *head, Error **errp);
 
 union qemu_plugin_cb_sig {
@@ -242,7 +242,7 @@ void qemu_plugin_user_postfork(bool is_child);
 static inline void qemu_plugin_add_opts(void)
 { }
 
-static inline void qemu_plugin_opt_parse(const char *optarg,
+static inline void qemu_plugin_opt_parse(const char *optstr,
                                          QemuPluginList *head)
 {
     error_report("plugin interface not enabled in this build");
diff --git a/plugins/loader.c b/plugins/loader.c
index 809f3f9b13..734c11cae0 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -140,12 +140,12 @@ static int plugin_add(void *opaque, const char *name, const char *value,
     return 0;
 }
 
-void qemu_plugin_opt_parse(const char *optarg, QemuPluginList *head)
+void qemu_plugin_opt_parse(const char *optstr, QemuPluginList *head)
 {
     struct qemu_plugin_parse_arg arg;
     QemuOpts *opts;
 
-    opts = qemu_opts_parse_noisily(qemu_find_opts("plugin"), optarg, true);
+    opts = qemu_opts_parse_noisily(qemu_find_opts("plugin"), optstr, true);
     if (opts == NULL) {
         exit(1);
     }
-- 
2.41.0

Re: [PATCH v3 05/16] plugins/loader: Clean up global variable shadowing

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Fix:
>
>   include/qemu/plugin.h:245:54: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>   static inline void qemu_plugin_opt_parse(const char *optarg,
>                                                        ^
>   /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>   extern char *optarg;                    /* getopt(3) external
>   variables */

My same raised eyebrows are how exactly getopt.h is getting included but
anyway:

Acked-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH v3 06/16] qemu-img: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  qemu-img.c:247:46: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static bool is_valid_option_list(const char *optarg)
                                               ^
  qemu-img.c:265:53: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static int accumulate_options(char **options, char *optarg)
                                                      ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
  extern char *optarg;                    /* getopt(3) external variables */
               ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 qemu-img.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index a48edb7101..6068ab0d27 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -235,25 +235,25 @@ void help(void)
 }
 
 /*
- * Is @optarg safe for accumulate_options()?
+ * Is @list safe for accumulate_options()?
  * It is when multiple of them can be joined together separated by ','.
- * To make that work, @optarg must not start with ',' (or else a
+ * To make that work, @list must not start with ',' (or else a
  * separating ',' preceding it gets escaped), and it must not end with
  * an odd number of ',' (or else a separating ',' following it gets
  * escaped), or be empty (or else a separating ',' preceding it can
  * escape a separating ',' following it).
  * 
  */
-static bool is_valid_option_list(const char *optarg)
+static bool is_valid_option_list(const char *list)
 {
-    size_t len = strlen(optarg);
+    size_t len = strlen(list);
     size_t i;
 
-    if (!optarg[0] || optarg[0] == ',') {
+    if (!list[0] || list[0] == ',') {
         return false;
     }
 
-    for (i = len; i > 0 && optarg[i - 1] == ','; i--) {
+    for (i = len; i > 0 && list[i - 1] == ','; i--) {
     }
     if ((len - i) % 2) {
         return false;
@@ -262,19 +262,19 @@ static bool is_valid_option_list(const char *optarg)
     return true;
 }
 
-static int accumulate_options(char **options, char *optarg)
+static int accumulate_options(char **options, char *list)
 {
     char *new_options;
 
-    if (!is_valid_option_list(optarg)) {
-        error_report("Invalid option list: %s", optarg);
+    if (!is_valid_option_list(list)) {
+        error_report("Invalid option list: %s", list);
         return -1;
     }
 
     if (!*options) {
-        *options = g_strdup(optarg);
+        *options = g_strdup(list);
     } else {
-        new_options = g_strdup_printf("%s,%s", *options, optarg);
+        new_options = g_strdup_printf("%s,%s", *options, list);
         g_free(*options);
         *options = new_options;
     }
-- 
2.41.0

Re: [PATCH v3 06/16] qemu-img: Clean up global variable shadowing

[Eric Blake]

On Wed, Oct 04, 2023 at 02:00:09PM +0200, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>   qemu-img.c:247:46: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>   static bool is_valid_option_list(const char *optarg)
>                                                ^
>   qemu-img.c:265:53: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>   static int accumulate_options(char **options, char *optarg)
>                                                       ^
>   /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>   extern char *optarg;                    /* getopt(3) external variables */
>                ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  qemu-img.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

[PATCH v3 07/16] qemu-io: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  qemu-io.c:478:36: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void add_user_command(char *optarg)
                                     ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
  extern char *optarg;                    /* getopt(3) external variables */
               ^

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

diff --git a/qemu-io.c b/qemu-io.c
index 2bd7bfb650..050c70835f 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -475,10 +475,10 @@ static int command_loop(void)
     return last_error;
 }
 
-static void add_user_command(char *optarg)
+static void add_user_command(char *user_cmd)
 {
     cmdline = g_renew(char *, cmdline, ++ncmdline);
-    cmdline[ncmdline-1] = optarg;
+    cmdline[ncmdline - 1] = user_cmd;
 }
 
 static void reenable_tty_echo(void)
-- 
2.41.0

Re: [PATCH v3 07/16] qemu-io: Clean up global variable shadowing

[Eric Blake]

On Wed, Oct 04, 2023 at 02:00:10PM +0200, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>   qemu-io.c:478:36: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>   static void add_user_command(char *optarg)
>                                      ^
>   /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>   extern char *optarg;                    /* getopt(3) external variables */
>                ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  qemu-io.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

[PATCH v3 08/16] qom/object_interfaces: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  qom/object_interfaces.c:262:53: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp)
                                                      ^
  qom/object_interfaces.c:298:46: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  bool user_creatable_add_from_str(const char *optarg, Error **errp)
                                               ^
  qom/object_interfaces.c:313:49: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  void user_creatable_process_cmdline(const char *optarg)
                                                  ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
  extern char *optarg;                    /* getopt(3) external variables */
               ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qom/object_interfaces.h | 16 ++++++++--------
 qom/object_interfaces.c         | 16 ++++++++--------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 81541e2080..02b11a7ef0 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -99,7 +99,7 @@ void user_creatable_add_qapi(ObjectOptions *options, Error **errp);
 
 /**
  * user_creatable_parse_str:
- * @optarg: the object definition string as passed on the command line
+ * @str: the object definition string as passed on the command line
  * @errp: if an error occurs, a pointer to an area to store the error
  *
  * Parses the option for the user creatable object with a keyval parser and
@@ -110,14 +110,14 @@ void user_creatable_add_qapi(ObjectOptions *options, Error **errp);
  * Returns: ObjectOptions on success, NULL when an error occurred (*errp is set
  * then) or help was printed (*errp is not set).
  */
-ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp);
+ObjectOptions *user_creatable_parse_str(const char *str, Error **errp);
 
 /**
  * user_creatable_add_from_str:
- * @optarg: the object definition string as passed on the command line
+ * @str: the object definition string as passed on the command line
  * @errp: if an error occurs, a pointer to an area to store the error
  *
- * Create an instance of the user creatable object by parsing optarg
+ * Create an instance of the user creatable object by parsing @str
  * with a keyval parser and implicit key 'qom-type', converting the
  * result to ObjectOptions and calling into qmp_object_add().
  *
@@ -126,13 +126,13 @@ ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp);
  * Returns: true when an object was successfully created, false when an error
  * occurred (*errp is set then) or help was printed (*errp is not set).
  */
-bool user_creatable_add_from_str(const char *optarg, Error **errp);
+bool user_creatable_add_from_str(const char *str, Error **errp);
 
 /**
  * user_creatable_process_cmdline:
- * @optarg: the object definition string as passed on the command line
+ * @cmdline: the object definition string as passed on the command line
  *
- * Create an instance of the user creatable object by parsing optarg
+ * Create an instance of the user creatable object by parsing @cmdline
  * with a keyval parser and implicit key 'qom-type', converting the
  * result to ObjectOptions and calling into qmp_object_add().
  *
@@ -141,7 +141,7 @@ bool user_creatable_add_from_str(const char *optarg, Error **errp);
  * This function is only meant to be called during command line parsing.
  * It exits the process on failure or after printing help.
  */
-void user_creatable_process_cmdline(const char *optarg);
+void user_creatable_process_cmdline(const char *cmdline);
 
 /**
  * user_creatable_print_help:
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 7d31589b04..e0833c8bfe 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -259,7 +259,7 @@ static void user_creatable_print_help_from_qdict(QDict *args)
     }
 }
 
-ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp)
+ObjectOptions *user_creatable_parse_str(const char *str, Error **errp)
 {
     ERRP_GUARD();
     QObject *obj;
@@ -267,14 +267,14 @@ ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp)
     Visitor *v;
     ObjectOptions *options;
 
-    if (optarg[0] == '{') {
-        obj = qobject_from_json(optarg, errp);
+    if (str[0] == '{') {
+        obj = qobject_from_json(str, errp);
         if (!obj) {
             return NULL;
         }
         v = qobject_input_visitor_new(obj);
     } else {
-        QDict *args = keyval_parse(optarg, "qom-type", &help, errp);
+        QDict *args = keyval_parse(str, "qom-type", &help, errp);
         if (*errp) {
             return NULL;
         }
@@ -295,12 +295,12 @@ ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp)
     return options;
 }
 
-bool user_creatable_add_from_str(const char *optarg, Error **errp)
+bool user_creatable_add_from_str(const char *str, Error **errp)
 {
     ERRP_GUARD();
     ObjectOptions *options;
 
-    options = user_creatable_parse_str(optarg, errp);
+    options = user_creatable_parse_str(str, errp);
     if (!options) {
         return false;
     }
@@ -310,9 +310,9 @@ bool user_creatable_add_from_str(const char *optarg, Error **errp)
     return !*errp;
 }
 
-void user_creatable_process_cmdline(const char *optarg)
+void user_creatable_process_cmdline(const char *cmdline)
 {
-    if (!user_creatable_add_from_str(optarg, &error_fatal)) {
+    if (!user_creatable_add_from_str(cmdline, &error_fatal)) {
         /* Help was printed */
         exit(EXIT_SUCCESS);
     }
-- 
2.41.0

[PATCH v3 09/16] semihosting: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  semihosting/config.c:134:49: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  int qemu_semihosting_config_options(const char *optarg)
                                                  ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
  extern char *optarg;                    /* getopt(3) external variables */
               ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/semihosting/semihost.h | 2 +-
 semihosting/config.c           | 8 ++++----
 stubs/semihost.c               | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/semihosting/semihost.h b/include/semihosting/semihost.h
index efd2efa25a..97d2a2ba99 100644
--- a/include/semihosting/semihost.h
+++ b/include/semihosting/semihost.h
@@ -66,7 +66,7 @@ const char *semihosting_get_cmdline(void);
 void semihosting_arg_fallback(const char *file, const char *cmd);
 /* for vl.c hooks */
 void qemu_semihosting_enable(void);
-int qemu_semihosting_config_options(const char *opt);
+int qemu_semihosting_config_options(const char *optstr);
 void qemu_semihosting_chardev_init(void);
 void qemu_semihosting_console_init(Chardev *);
 #endif /* CONFIG_USER_ONLY */
diff --git a/semihosting/config.c b/semihosting/config.c
index 8ca569735d..e826457733 100644
--- a/semihosting/config.c
+++ b/semihosting/config.c
@@ -131,10 +131,10 @@ void qemu_semihosting_enable(void)
     semihosting.target = SEMIHOSTING_TARGET_AUTO;
 }
 
-int qemu_semihosting_config_options(const char *optarg)
+int qemu_semihosting_config_options(const char *optstr)
 {
     QemuOptsList *opt_list = qemu_find_opts("semihosting-config");
-    QemuOpts *opts = qemu_opts_parse_noisily(opt_list, optarg, false);
+    QemuOpts *opts = qemu_opts_parse_noisily(opt_list, optstr, false);
 
     semihosting.enabled = true;
 
@@ -155,7 +155,7 @@ int qemu_semihosting_config_options(const char *optarg)
                 semihosting.target = SEMIHOSTING_TARGET_AUTO;
             } else {
                 error_report("unsupported semihosting-config %s",
-                             optarg);
+                             optstr);
                 return 1;
             }
         } else {
@@ -165,7 +165,7 @@ int qemu_semihosting_config_options(const char *optarg)
         qemu_opt_foreach(opts, add_semihosting_arg,
                          &semihosting, NULL);
     } else {
-        error_report("unsupported semihosting-config %s", optarg);
+        error_report("unsupported semihosting-config %s", optstr);
         return 1;
     }
 
diff --git a/stubs/semihost.c b/stubs/semihost.c
index aad7a70353..b3c61935b3 100644
--- a/stubs/semihost.c
+++ b/stubs/semihost.c
@@ -36,7 +36,7 @@ void qemu_semihosting_enable(void)
 {
 }
 
-int qemu_semihosting_config_options(const char *optarg)
+int qemu_semihosting_config_options(const char *optstr)
 {
     return 1;
 }
-- 
2.41.0

Re: [PATCH v3 09/16] semihosting: Clean up global variable shadowing

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Fix:
>
>   semihosting/config.c:134:49: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>   int qemu_semihosting_config_options(const char *optarg)
>                                                   ^
>   /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>   extern char *optarg;                    /* getopt(3) external
>   variables */

I'm going to assume the getopt.h is somehow swept up by osdep.h?

Anyway:

Acked-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v3 09/16] semihosting: Clean up global variable shadowing

[Thomas Huth]

On 04/10/2023 14.16, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> Fix:
>>
>>    semihosting/config.c:134:49: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>>    int qemu_semihosting_config_options(const char *optarg)
>>                                                    ^
>>    /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>>    extern char *optarg;                    /* getopt(3) external
>>    variables */
> 
> I'm going to assume the getopt.h is somehow swept up by osdep.h?
> 
> Anyway:
> 
> Acked-by: Alex Bennée <alex.bennee@linaro.org>

Could we maybe rather remove getopt.h from osdep.h instead of renaming this 
everywhere? getopt.h should only be required by some few files, so including 
this in osdep.h seems exaggerated, IMHO.

  Thomas

Re: [PATCH v3 09/16] semihosting: Clean up global variable shadowing

[Markus Armbruster]

Alex Bennée <alex.bennee@linaro.org> writes:

> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
>> Fix:
>>
>>   semihosting/config.c:134:49: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>>   int qemu_semihosting_config_options(const char *optarg)
>>                                                   ^
>>   /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>>   extern char *optarg;                    /* getopt(3) external
>>   variables */
>
> I'm going to assume the getopt.h is somehow swept up by osdep.h?

It comes from unitstd.h via osdep.h.  getopt.h provides getopt_long() &
friends.
>
> Anyway:
>
> Acked-by: Alex Bennée <alex.bennee@linaro.org>

[PATCH v3 10/16] ui/cocoa: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  ui/cocoa.m:346:20: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      QemuCocoaView *cocoaView = userInfo;
                     ^
  ui/cocoa.m:342:16: note: previous declaration is here
  QemuCocoaView *cocoaView;
                 ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 ui/cocoa.m | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index df6d13be38..15477288fd 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -343,9 +343,9 @@ - (void) raiseAllKeys;
 
 static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEventRef cgEvent, void *userInfo)
 {
-    QemuCocoaView *cocoaView = userInfo;
+    QemuCocoaView *view = userInfo;
     NSEvent *event = [NSEvent eventWithCGEvent:cgEvent];
-    if ([cocoaView isMouseGrabbed] && [cocoaView handleEvent:event]) {
+    if ([view isMouseGrabbed] && [view handleEvent:event]) {
         COCOA_DEBUG("Global events tap: qemu handled the event, capturing!\n");
         return NULL;
     }
-- 
2.41.0

Re: [PATCH v3 10/16] ui/cocoa: Clean up global variable shadowing

[Akihiko Odaki]

On 2023/10/04 21:00, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>    ui/cocoa.m:346:20: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>        QemuCocoaView *cocoaView = userInfo;
>                       ^
>    ui/cocoa.m:342:16: note: previous declaration is here
>    QemuCocoaView *cocoaView;
>                   ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   ui/cocoa.m | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index df6d13be38..15477288fd 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -343,9 +343,9 @@ - (void) raiseAllKeys;
>   
>   static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEventRef cgEvent, void *userInfo)
>   {
> -    QemuCocoaView *cocoaView = userInfo;
> +    QemuCocoaView *view = userInfo;
>       NSEvent *event = [NSEvent eventWithCGEvent:cgEvent];
> -    if ([cocoaView isMouseGrabbed] && [cocoaView handleEvent:event]) {
> +    if ([view isMouseGrabbed] && [view handleEvent:event]) {
>           COCOA_DEBUG("Global events tap: qemu handled the event, capturing!\n");
>           return NULL;
>       }

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

[PATCH v3 11/16] util/cutils: Clean up global variable shadowing in get_relocated_path()

[Philippe Mathieu-Daudé]

Fix:

  util/cutils.c:1147:17: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      const char *exec_dir = qemu_get_exec_dir();
                  ^
  util/cutils.c:1035:20: note: previous declaration is here
  static const char *exec_dir;
                     ^

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

diff --git a/util/cutils.c b/util/cutils.c
index 25373198ad..b44718a6a2 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -1144,7 +1144,6 @@ char *get_relocated_path(const char *dir)
 {
     size_t prefix_len = strlen(CONFIG_PREFIX);
     const char *bindir = CONFIG_BINDIR;
-    const char *exec_dir = qemu_get_exec_dir();
     GString *result;
     int len_dir, len_bindir;
 
-- 
2.41.0

Re: [PATCH v3 11/16] util/cutils: Clean up global variable shadowing in get_relocated_path()

[Eric Blake]

On Wed, Oct 04, 2023 at 02:00:14PM +0200, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>   util/cutils.c:1147:17: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>       const char *exec_dir = qemu_get_exec_dir();
>                   ^
>   util/cutils.c:1035:20: note: previous declaration is here
>   static const char *exec_dir;
>                      ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  util/cutils.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/util/cutils.c b/util/cutils.c
> index 25373198ad..b44718a6a2 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -1144,7 +1144,6 @@ char *get_relocated_path(const char *dir)
>  {
>      size_t prefix_len = strlen(CONFIG_PREFIX);
>      const char *bindir = CONFIG_BINDIR;
> -    const char *exec_dir = qemu_get_exec_dir();
>      GString *result;
>      int len_dir, len_bindir;
>  
> --

Took me a few seconds to see it, but since we have this just a few lines before:

const char *qemu_get_exec_dir(void)
{
    return exec_dir;
}

the deletion of the redundant local variable is just fine.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

[PATCH v3 12/16] util/guest-random: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  util/guest-random.c:90:45: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  int qemu_guest_random_seed_main(const char *optarg, Error **errp)
                                              ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
  extern char *optarg;                    /* getopt(3) external variables */
               ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qemu/guest-random.h | 8 ++++----
 util/guest-random.c         | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/qemu/guest-random.h b/include/qemu/guest-random.h
index 09ff9c2236..5060d49d60 100644
--- a/include/qemu/guest-random.h
+++ b/include/qemu/guest-random.h
@@ -13,16 +13,16 @@
 #define QEMU_GUEST_RANDOM_H
 
 /**
- * qemu_guest_random_seed_main(const char *optarg, Error **errp)
- * @optarg: a non-NULL pointer to a C string
+ * qemu_guest_random_seed_main(const char *seedstr, Error **errp)
+ * @seedstr: a non-NULL pointer to a C string
  * @errp: an error indicator
  *
- * The @optarg value is that which accompanies the -seed argument.
+ * The @seedstr value is that which accompanies the -seed argument.
  * This forces qemu_guest_getrandom into deterministic mode.
  *
  * Returns 0 on success, < 0 on failure while setting *errp.
  */
-int qemu_guest_random_seed_main(const char *optarg, Error **errp);
+int qemu_guest_random_seed_main(const char *seedstr, Error **errp);
 
 /**
  * qemu_guest_random_seed_thread_part1(void)
diff --git a/util/guest-random.c b/util/guest-random.c
index 9465dda085..33607d5ff2 100644
--- a/util/guest-random.c
+++ b/util/guest-random.c
@@ -87,11 +87,11 @@ void qemu_guest_random_seed_thread_part2(uint64_t seed)
     }
 }
 
-int qemu_guest_random_seed_main(const char *optarg, Error **errp)
+int qemu_guest_random_seed_main(const char *seedstr, Error **errp)
 {
     uint64_t seed;
-    if (parse_uint_full(optarg, 0, &seed)) {
-        error_setg(errp, "Invalid seed number: %s", optarg);
+    if (parse_uint_full(seedstr, 0, &seed)) {
+        error_setg(errp, "Invalid seed number: %s", seedstr);
         return -1;
     } else {
         deterministic = true;
-- 
2.41.0

[PATCH v3 13/16] semihosting/arm-compat: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  semihosting/arm-compat-semi.c: In function ‘do_common_semihosting’:
  semihosting/arm-compat-semi.c:379:13: warning: declaration of ‘ret’ shadows a previous local [-Wshadow=local]
    379 |         int ret, err = 0;
        |             ^~~
  semihosting/arm-compat-semi.c:370:14: note: shadowed declaration is here
    370 |     uint32_t ret;
        |              ^~~
  semihosting/arm-compat-semi.c:682:27: warning: declaration of ‘ret’ shadows a previous local [-Wshadow=local]
    682 |                 abi_ulong ret;
        |                           ^~~
  semihosting/arm-compat-semi.c:370:9: note: shadowed declaration is here
    370 |     int ret;
        |         ^~~

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 semihosting/arm-compat-semi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
index 564fe17f75..0033a1e018 100644
--- a/semihosting/arm-compat-semi.c
+++ b/semihosting/arm-compat-semi.c
@@ -367,7 +367,6 @@ void do_common_semihosting(CPUState *cs)
     target_ulong ul_ret;
     char * s;
     int nr;
-    uint32_t ret;
     int64_t elapsed;
 
     nr = common_semi_arg(cs, 0) & 0xffffffffU;
@@ -725,6 +724,9 @@ void do_common_semihosting(CPUState *cs)
 
     case TARGET_SYS_EXIT:
     case TARGET_SYS_EXIT_EXTENDED:
+    {
+        uint32_t ret;
+
         if (common_semi_sys_exit_extended(cs, nr)) {
             /*
              * The A64 version of SYS_EXIT takes a parameter block,
@@ -752,6 +754,7 @@ void do_common_semihosting(CPUState *cs)
         }
         gdb_exit(ret);
         exit(ret);
+    }
 
     case TARGET_SYS_ELAPSED:
         elapsed = get_clock() - clock_start;
-- 
2.41.0

Re: [PATCH v3 13/16] semihosting/arm-compat: Clean up local variable shadowing

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Fix:
>
>   semihosting/arm-compat-semi.c: In function ‘do_common_semihosting’:
>   semihosting/arm-compat-semi.c:379:13: warning: declaration of ‘ret’ shadows a previous local [-Wshadow=local]
>     379 |         int ret, err = 0;
>         |             ^~~
>   semihosting/arm-compat-semi.c:370:14: note: shadowed declaration is here
>     370 |     uint32_t ret;
>         |              ^~~
>   semihosting/arm-compat-semi.c:682:27: warning: declaration of ‘ret’
> shadows a previous local [-Wshadow=local]
>     682 |                 abi_ulong ret;
>         |                           ^~~
>   semihosting/arm-compat-semi.c:370:9: note: shadowed declaration is here
>     370 |     int ret;
>         |         ^~~
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  semihosting/arm-compat-semi.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
> index 564fe17f75..0033a1e018 100644
> --- a/semihosting/arm-compat-semi.c
> +++ b/semihosting/arm-compat-semi.c
> @@ -367,7 +367,6 @@ void do_common_semihosting(CPUState *cs)
>      target_ulong ul_ret;
>      char * s;
>      int nr;
> -    uint32_t ret;
>      int64_t elapsed;
>  
>      nr = common_semi_arg(cs, 0) & 0xffffffffU;
> @@ -725,6 +724,9 @@ void do_common_semihosting(CPUState *cs)
>  
>      case TARGET_SYS_EXIT:
>      case TARGET_SYS_EXIT_EXTENDED:
> +    {
> +        uint32_t ret;
> +

I suspect this could just as well be an int with an explicit cast for ret = arg1
because the consumers are all expecting int anyway.

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v3 13/16] semihosting/arm-compat: Clean up local variable shadowing

[Markus Armbruster]

Alex Bennée <alex.bennee@linaro.org> writes:

> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
>> Fix:
>>
>>   semihosting/arm-compat-semi.c: In function ‘do_common_semihosting’:
>>   semihosting/arm-compat-semi.c:379:13: warning: declaration of ‘ret’ shadows a previous local [-Wshadow=local]
>>     379 |         int ret, err = 0;
>>         |             ^~~
>>   semihosting/arm-compat-semi.c:370:14: note: shadowed declaration is here
>>     370 |     uint32_t ret;
>>         |              ^~~
>>   semihosting/arm-compat-semi.c:682:27: warning: declaration of ‘ret’
>> shadows a previous local [-Wshadow=local]
>>     682 |                 abi_ulong ret;
>>         |                           ^~~
>>   semihosting/arm-compat-semi.c:370:9: note: shadowed declaration is here
>>     370 |     int ret;
>>         |         ^~~
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>  semihosting/arm-compat-semi.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
>> index 564fe17f75..0033a1e018 100644
>> --- a/semihosting/arm-compat-semi.c
>> +++ b/semihosting/arm-compat-semi.c
>> @@ -367,7 +367,6 @@ void do_common_semihosting(CPUState *cs)
>>      target_ulong ul_ret;
>>      char * s;
>>      int nr;
>> -    uint32_t ret;
>>      int64_t elapsed;
>>  
>>      nr = common_semi_arg(cs, 0) & 0xffffffffU;
>> @@ -725,6 +724,9 @@ void do_common_semihosting(CPUState *cs)
>>  
>>      case TARGET_SYS_EXIT:
>>      case TARGET_SYS_EXIT_EXTENDED:
>> +    {
>> +        uint32_t ret;
>> +
>
> I suspect this could just as well be an int with an explicit cast for ret = arg1
> because the consumers are all expecting int anyway.
>
> Otherwise:
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Philippe, would you like to follow up on Alex's suspicion, or would you
like me to queue the patch as is?

Re: [PATCH v3 13/16] semihosting/arm-compat: Clean up local variable shadowing

[Philippe Mathieu-Daudé]

On 6/10/23 11:14, Markus Armbruster wrote:
> Alex Bennée <alex.bennee@linaro.org> writes:
> 
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>>> Fix:
>>>
>>>    semihosting/arm-compat-semi.c: In function ‘do_common_semihosting’:
>>>    semihosting/arm-compat-semi.c:379:13: warning: declaration of ‘ret’ shadows a previous local [-Wshadow=local]
>>>      379 |         int ret, err = 0;
>>>          |             ^~~
>>>    semihosting/arm-compat-semi.c:370:14: note: shadowed declaration is here
>>>      370 |     uint32_t ret;
>>>          |              ^~~
>>>    semihosting/arm-compat-semi.c:682:27: warning: declaration of ‘ret’
>>> shadows a previous local [-Wshadow=local]
>>>      682 |                 abi_ulong ret;
>>>          |                           ^~~
>>>    semihosting/arm-compat-semi.c:370:9: note: shadowed declaration is here
>>>      370 |     int ret;
>>>          |         ^~~
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   semihosting/arm-compat-semi.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
>>> index 564fe17f75..0033a1e018 100644
>>> --- a/semihosting/arm-compat-semi.c
>>> +++ b/semihosting/arm-compat-semi.c
>>> @@ -367,7 +367,6 @@ void do_common_semihosting(CPUState *cs)
>>>       target_ulong ul_ret;
>>>       char * s;
>>>       int nr;
>>> -    uint32_t ret;
>>>       int64_t elapsed;
>>>   
>>>       nr = common_semi_arg(cs, 0) & 0xffffffffU;
>>> @@ -725,6 +724,9 @@ void do_common_semihosting(CPUState *cs)
>>>   
>>>       case TARGET_SYS_EXIT:
>>>       case TARGET_SYS_EXIT_EXTENDED:
>>> +    {
>>> +        uint32_t ret;
>>> +
>>
>> I suspect this could just as well be an int with an explicit cast for ret = arg1
>> because the consumers are all expecting int anyway.
>>
>> Otherwise:
>>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 
> Philippe, would you like to follow up on Alex's suspicion, or would you
> like me to queue the patch as is?

Please queue as is. The signed conversion is done here (well
documented):
https://lore.kernel.org/qemu-devel/20231005062610.57351-1-philmd@linaro.org/

[PATCH v3 14/16] softmmu/vl: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  softmmu/vl.c:1069:44: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void parse_display_qapi(const char *optarg)
                                             ^
  softmmu/vl.c:1224:39: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void monitor_parse(const char *optarg, const char *mode, bool pretty)
                                        ^
  softmmu/vl.c:1634:17: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
      const char *optarg = qdict_get_try_str(qdict, "type");
                  ^
  softmmu/vl.c:1784:45: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  static void object_option_parse(const char *optarg)
                                              ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
  extern char *optarg;                    /* getopt(3) external variables */
               ^

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

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 98e071e63b..ae1ff9887d 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1066,12 +1066,12 @@ static void select_vgahw(const MachineClass *machine_class, const char *p)
     }
 }
 
-static void parse_display_qapi(const char *optarg)
+static void parse_display_qapi(const char *optstr)
 {
     DisplayOptions *opts;
     Visitor *v;
 
-    v = qobject_input_visitor_new_str(optarg, "type", &error_fatal);
+    v = qobject_input_visitor_new_str(optstr, "type", &error_fatal);
 
     visit_type_DisplayOptions(v, NULL, &opts, &error_fatal);
     QAPI_CLONE_MEMBERS(DisplayOptions, &dpy, opts);
@@ -1221,21 +1221,21 @@ static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
     return monitor_init_opts(opts, errp);
 }
 
-static void monitor_parse(const char *optarg, const char *mode, bool pretty)
+static void monitor_parse(const char *str, const char *mode, bool pretty)
 {
     static int monitor_device_index = 0;
     QemuOpts *opts;
     const char *p;
     char label[32];
 
-    if (strstart(optarg, "chardev:", &p)) {
+    if (strstart(str, "chardev:", &p)) {
         snprintf(label, sizeof(label), "%s", p);
     } else {
         snprintf(label, sizeof(label), "compat_monitor%d",
                  monitor_device_index);
-        opts = qemu_chr_parse_compat(label, optarg, true);
+        opts = qemu_chr_parse_compat(label, str, true);
         if (!opts) {
-            error_report("parse error: %s", optarg);
+            error_report("parse error: %s", str);
             exit(1);
         }
     }
@@ -1631,13 +1631,13 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
 
 static MachineClass *select_machine(QDict *qdict, Error **errp)
 {
-    const char *optarg = qdict_get_try_str(qdict, "type");
+    const char *machine_type = qdict_get_try_str(qdict, "type");
     GSList *machines = object_class_get_list(TYPE_MACHINE, false);
     MachineClass *machine_class;
     Error *local_err = NULL;
 
-    if (optarg) {
-        machine_class = find_machine(optarg, machines);
+    if (machine_type) {
+        machine_class = find_machine(machine_type, machines);
         qdict_del(qdict, "type");
         if (!machine_class) {
             error_setg(&local_err, "unsupported machine type");
@@ -1781,20 +1781,20 @@ static void object_option_add_visitor(Visitor *v)
     QTAILQ_INSERT_TAIL(&object_opts, opt, next);
 }
 
-static void object_option_parse(const char *optarg)
+static void object_option_parse(const char *optstr)
 {
     QemuOpts *opts;
     const char *type;
     Visitor *v;
 
-    if (optarg[0] == '{') {
-        QObject *obj = qobject_from_json(optarg, &error_fatal);
+    if (optstr[0] == '{') {
+        QObject *obj = qobject_from_json(optstr, &error_fatal);
 
         v = qobject_input_visitor_new(obj);
         qobject_unref(obj);
     } else {
         opts = qemu_opts_parse_noisily(qemu_find_opts("object"),
-                                       optarg, true);
+                                       optstr, true);
         if (!opts) {
             exit(1);
         }
-- 
2.41.0

Re: [PATCH v3 14/16] softmmu/vl: Clean up global variable shadowing

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Fix:
>
>   softmmu/vl.c:1069:44: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>   static void parse_display_qapi(const char *optarg)
>                                              ^
>   softmmu/vl.c:1224:39: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>   static void monitor_parse(const char *optarg, const char *mode, bool pretty)
>                                         ^
>   softmmu/vl.c:1634:17: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>       const char *optarg = qdict_get_try_str(qdict, "type");
>                   ^
>   softmmu/vl.c:1784:45: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>   static void object_option_parse(const char *optarg)
>                                               ^
>   /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>   extern char *optarg;                    /* getopt(3) external variables */
>                ^
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

How much we care about the shadowing is unclear, but that doesn't matter
if the patches make sense even if we pretend global @optarg doesn't
exist.  Let's check that.

> ---
>  softmmu/vl.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 98e071e63b..ae1ff9887d 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1066,12 +1066,12 @@ static void select_vgahw(const MachineClass *machine_class, const char *p)
>      }
>  }
>  
> -static void parse_display_qapi(const char *optarg)
> +static void parse_display_qapi(const char *optstr)
>  {
>      DisplayOptions *opts;
>      Visitor *v;
>  
> -    v = qobject_input_visitor_new_str(optarg, "type", &error_fatal);
> +    v = qobject_input_visitor_new_str(optstr, "type", &error_fatal);
>  
>      visit_type_DisplayOptions(v, NULL, &opts, &error_fatal);
>      QAPI_CLONE_MEMBERS(DisplayOptions, &dpy, opts);

The actual argument is a string that is either JSON or KEY=VALUE,...
The fact that it's always an option argument now (actually the value of
global @optarg) is irrelevant here.

parse_display_qapi() passes its parameter to
qobject_input_visitor_new_str() parameter @str, which passes it to
qobject_from_json() parameter @string if JSON, or else to keyval_parse()
parameter @params.

I'd rename @optarg to @str here, like you do in the next hunk, to not
suggest a connection to CLI.  Not a demand.

> @@ -1221,21 +1221,21 @@ static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
>      return monitor_init_opts(opts, errp);
>  }
>  
> -static void monitor_parse(const char *optarg, const char *mode, bool pretty)
> +static void monitor_parse(const char *str, const char *mode, bool pretty)
>  {
>      static int monitor_device_index = 0;
>      QemuOpts *opts;
>      const char *p;
>      char label[32];
>  
> -    if (strstart(optarg, "chardev:", &p)) {
> +    if (strstart(str, "chardev:", &p)) {
>          snprintf(label, sizeof(label), "%s", p);
>      } else {
>          snprintf(label, sizeof(label), "compat_monitor%d",
>                   monitor_device_index);
> -        opts = qemu_chr_parse_compat(label, optarg, true);
> +        opts = qemu_chr_parse_compat(label, str, true);
>          if (!opts) {
> -            error_report("parse error: %s", optarg);
> +            error_report("parse error: %s", str);
>              exit(1);
>          }
>      }

The actual argument is either @optarg or a string literal, but that's
again irrelevant here.

> @@ -1631,13 +1631,13 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
>  
>  static MachineClass *select_machine(QDict *qdict, Error **errp)
>  {
> -    const char *optarg = qdict_get_try_str(qdict, "type");
> +    const char *machine_type = qdict_get_try_str(qdict, "type");
>      GSList *machines = object_class_get_list(TYPE_MACHINE, false);
>      MachineClass *machine_class;
>      Error *local_err = NULL;
>  
> -    if (optarg) {
> -        machine_class = find_machine(optarg, machines);
> +    if (machine_type) {
> +        machine_class = find_machine(machine_type, machines);
>          qdict_del(qdict, "type");
>          if (!machine_class) {
>              error_setg(&local_err, "unsupported machine type");

Obvious improvement.

> @@ -1781,20 +1781,20 @@ static void object_option_add_visitor(Visitor *v)
>      QTAILQ_INSERT_TAIL(&object_opts, opt, next);
>  }
>  
> -static void object_option_parse(const char *optarg)
> +static void object_option_parse(const char *optstr)
>  {
>      QemuOpts *opts;
>      const char *type;
>      Visitor *v;
>  
> -    if (optarg[0] == '{') {
> -        QObject *obj = qobject_from_json(optarg, &error_fatal);
> +    if (optstr[0] == '{') {
> +        QObject *obj = qobject_from_json(optstr, &error_fatal);
>  
>          v = qobject_input_visitor_new(obj);
>          qobject_unref(obj);
>      } else {
>          opts = qemu_opts_parse_noisily(qemu_find_opts("object"),
> -                                       optarg, true);
> +                                       optstr, true);
>          if (!opts) {
>              exit(1);
>          }

Same argument as for parse_display_qapi(), and same suggestion.

If this goes though my tree, I can implement my two suggestions, if you
agree.

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v3 14/16] softmmu/vl: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

On 5/10/23 10:59, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> Fix:
>>
>>    softmmu/vl.c:1069:44: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>>    static void parse_display_qapi(const char *optarg)
>>                                               ^
>>    softmmu/vl.c:1224:39: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>>    static void monitor_parse(const char *optarg, const char *mode, bool pretty)
>>                                          ^
>>    softmmu/vl.c:1634:17: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>>        const char *optarg = qdict_get_try_str(qdict, "type");
>>                    ^
>>    softmmu/vl.c:1784:45: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>>    static void object_option_parse(const char *optarg)
>>                                                ^
>>    /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>>    extern char *optarg;                    /* getopt(3) external variables */
>>                 ^
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> How much we care about the shadowing is unclear, but that doesn't matter
> if the patches make sense even if we pretend global @optarg doesn't
> exist.  Let's check that.
> 
>> ---
>>   softmmu/vl.c | 26 +++++++++++++-------------
>>   1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index 98e071e63b..ae1ff9887d 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -1066,12 +1066,12 @@ static void select_vgahw(const MachineClass *machine_class, const char *p)
>>       }
>>   }
>>   
>> -static void parse_display_qapi(const char *optarg)
>> +static void parse_display_qapi(const char *optstr)
>>   {
>>       DisplayOptions *opts;
>>       Visitor *v;
>>   
>> -    v = qobject_input_visitor_new_str(optarg, "type", &error_fatal);
>> +    v = qobject_input_visitor_new_str(optstr, "type", &error_fatal);
>>   
>>       visit_type_DisplayOptions(v, NULL, &opts, &error_fatal);
>>       QAPI_CLONE_MEMBERS(DisplayOptions, &dpy, opts);
> 
> The actual argument is a string that is either JSON or KEY=VALUE,...
> The fact that it's always an option argument now (actually the value of
> global @optarg) is irrelevant here.
> 
> parse_display_qapi() passes its parameter to
> qobject_input_visitor_new_str() parameter @str, which passes it to
> qobject_from_json() parameter @string if JSON, or else to keyval_parse()
> parameter @params.
> 
> I'd rename @optarg to @str here, like you do in the next hunk, to not
> suggest a connection to CLI.  Not a demand.

OK.


>> -static void object_option_parse(const char *optarg)
>> +static void object_option_parse(const char *optstr)
>>   {
>>       QemuOpts *opts;
>>       const char *type;
>>       Visitor *v;
>>   
>> -    if (optarg[0] == '{') {
>> -        QObject *obj = qobject_from_json(optarg, &error_fatal);
>> +    if (optstr[0] == '{') {
>> +        QObject *obj = qobject_from_json(optstr, &error_fatal);
>>   
>>           v = qobject_input_visitor_new(obj);
>>           qobject_unref(obj);
>>       } else {
>>           opts = qemu_opts_parse_noisily(qemu_find_opts("object"),
>> -                                       optarg, true);
>> +                                       optstr, true);
>>           if (!opts) {
>>               exit(1);
>>           }
> 
> Same argument as for parse_display_qapi(), and same suggestion.
> 
> If this goes though my tree, I can implement my two suggestions, if you
> agree.

Sure, thank you!

> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

[PATCH v3 15/16] sysemu/tpm: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  softmmu/tpm.c:178:59: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
                                                            ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
  extern char *optarg;                    /* getopt(3) external variables */
               ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/sysemu/tpm.h | 2 +-
 softmmu/tpm.c        | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 66e3b45f30..1ee568b3b6 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -17,7 +17,7 @@
 
 #ifdef CONFIG_TPM
 
-int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
+int tpm_config_parse(QemuOptsList *opts_list, const char *optstr);
 int tpm_init(void);
 void tpm_cleanup(void);
 
diff --git a/softmmu/tpm.c b/softmmu/tpm.c
index 578563f05a..7164ea7ff1 100644
--- a/softmmu/tpm.c
+++ b/softmmu/tpm.c
@@ -175,15 +175,15 @@ int tpm_init(void)
  * Parse the TPM configuration options.
  * To display all available TPM backends the user may use '-tpmdev help'
  */
-int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
+int tpm_config_parse(QemuOptsList *opts_list, const char *optstr)
 {
     QemuOpts *opts;
 
-    if (!strcmp(optarg, "help")) {
+    if (!strcmp(optstr, "help")) {
         tpm_display_backend_drivers();
         return -1;
     }
-    opts = qemu_opts_parse_noisily(opts_list, optarg, true);
+    opts = qemu_opts_parse_noisily(opts_list, optstr, true);
     if (!opts) {
         return -1;
     }
-- 
2.41.0

Re: [PATCH v3 15/16] sysemu/tpm: Clean up global variable shadowing

[Stefan Berger]

On 10/4/23 08:00, Philippe Mathieu-Daudé wrote:
> Fix:
>
>    softmmu/tpm.c:178:59: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>    int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
>                                                              ^
>    /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>    extern char *optarg;                    /* getopt(3) external variables */
>                 ^
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>



> ---
>   include/sysemu/tpm.h | 2 +-
>   softmmu/tpm.c        | 6 +++---
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index 66e3b45f30..1ee568b3b6 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -17,7 +17,7 @@
>
>   #ifdef CONFIG_TPM
>
> -int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
> +int tpm_config_parse(QemuOptsList *opts_list, const char *optstr);
>   int tpm_init(void);
>   void tpm_cleanup(void);
>
> diff --git a/softmmu/tpm.c b/softmmu/tpm.c
> index 578563f05a..7164ea7ff1 100644
> --- a/softmmu/tpm.c
> +++ b/softmmu/tpm.c
> @@ -175,15 +175,15 @@ int tpm_init(void)
>    * Parse the TPM configuration options.
>    * To display all available TPM backends the user may use '-tpmdev help'
>    */
> -int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
> +int tpm_config_parse(QemuOptsList *opts_list, const char *optstr)
>   {
>       QemuOpts *opts;
>
> -    if (!strcmp(optarg, "help")) {
> +    if (!strcmp(optstr, "help")) {
>           tpm_display_backend_drivers();
>           return -1;
>       }
> -    opts = qemu_opts_parse_noisily(opts_list, optarg, true);
> +    opts = qemu_opts_parse_noisily(opts_list, optstr, true);
>       if (!opts) {
>           return -1;
>       }

[PATCH v3 16/16] trace/control: Clean up global variable shadowing

[Philippe Mathieu-Daudé]

Fix:

  trace/control.c:288:34: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
  void trace_opt_parse(const char *optarg)
                                   ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
  extern char *optarg;                    /* getopt(3) external variables */
               ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 trace/control.h | 4 ++--
 trace/control.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/trace/control.h b/trace/control.h
index dfd209edd8..6754bfe052 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -197,11 +197,11 @@ extern QemuOptsList qemu_trace_opts;
 
 /**
  * trace_opt_parse:
- * @optarg: A string argument of --trace command line argument
+ * @optstr: A string argument of --trace command line argument
  *
  * Initialize tracing subsystem.
  */
-void trace_opt_parse(const char *optarg);
+void trace_opt_parse(const char *optstr);
 
 /**
  * trace_get_vcpu_event_count:
diff --git a/trace/control.c b/trace/control.c
index 1a48a7e266..ef107829ac 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -285,10 +285,10 @@ bool trace_init_backends(void)
     return true;
 }
 
-void trace_opt_parse(const char *optarg)
+void trace_opt_parse(const char *optstr)
 {
     QemuOpts *opts = qemu_opts_parse_noisily(qemu_find_opts("trace"),
-                                             optarg, true);
+                                             optstr, true);
     if (!opts) {
         exit(1);
     }
-- 
2.41.0

Re: [PATCH v3 16/16] trace/control: Clean up global variable shadowing

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

On Wed, Oct 04, 2023 at 02:00:19PM +0200, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>   trace/control.c:288:34: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
>   void trace_opt_parse(const char *optarg)
>                                    ^
>   /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here
>   extern char *optarg;                    /* getopt(3) external variables */
>                ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  trace/control.h | 4 ++--
>  trace/control.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 00/16] (few more) Steps towards enabling -Wshadow

[Richard Henderson]

On 10/4/23 05:00, Philippe Mathieu-Daudé wrote:
> Philippe Mathieu-Daudé (16):
>    hw/audio/soundhw: Clean up global variable shadowing
>    hw/ide/ahci: Clean up local variable shadowing
>    net/net: Clean up global variable shadowing
>    os-posix: Clean up global variable shadowing
>    plugins/loader: Clean up global variable shadowing
>    qemu-img: Clean up global variable shadowing
>    qemu-io: Clean up global variable shadowing
>    qom/object_interfaces: Clean up global variable shadowing
>    semihosting: Clean up global variable shadowing
>    ui/cocoa: Clean up global variable shadowing
>    util/cutils: Clean up global variable shadowing in
>      get_relocated_path()
>    util/guest-random: Clean up global variable shadowing
>    semihosting/arm-compat: Clean up local variable shadowing
>    softmmu/vl: Clean up global variable shadowing
>    sysemu/tpm: Clean up global variable shadowing
>    trace/control: Clean up global variable shadowing

Series:
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v3 00/16] (few more) Steps towards enabling -Wshadow

[Markus Armbruster]

Queued, thanks!

Re: [PATCH v3 00/16] (few more) Steps towards enabling -Wshadow

[Markus Armbruster]

Queued.  Thanks!