qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v3 14/16] softmmu/vl: Clean up global variable shadowing
Date: Thu, 5 Oct 2023 12:49:19 +0200
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.15.1

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>





reply via email to

[Prev in Thread] Current Thread [Next in Thread]