[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: |
Markus Armbruster |
Subject: |
Re: [PATCH v3 14/16] softmmu/vl: Clean up global variable shadowing |
Date: |
Thu, 05 Oct 2023 10:59:41 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
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 03/16] net/net: Clean up global variable shadowing, (continued)
- [PATCH v3 06/16] qemu-img: Clean up global variable shadowing, Philippe Mathieu-Daudé, 2023/10/04
- [PATCH v3 07/16] qemu-io: Clean up global variable shadowing, Philippe Mathieu-Daudé, 2023/10/04
- [PATCH v3 10/16] ui/cocoa: Clean up global variable shadowing, Philippe Mathieu-Daudé, 2023/10/04
- [PATCH v3 11/16] util/cutils: Clean up global variable shadowing in get_relocated_path(), Philippe Mathieu-Daudé, 2023/10/04
- [PATCH v3 14/16] softmmu/vl: Clean up global variable shadowing, Philippe Mathieu-Daudé, 2023/10/04
- Re: [PATCH v3 14/16] softmmu/vl: Clean up global variable shadowing,
Markus Armbruster <=
- [PATCH v3 13/16] semihosting/arm-compat: Clean up local variable shadowing, Philippe Mathieu-Daudé, 2023/10/04
- [PATCH v3 16/16] trace/control: Clean up global variable shadowing, Philippe Mathieu-Daudé, 2023/10/04
- Re: [PATCH v3 00/16] (few more) Steps towards enabling -Wshadow, Richard Henderson, 2023/10/04
- Re: [PATCH v3 00/16] (few more) Steps towards enabling -Wshadow, Markus Armbruster, 2023/10/06
- Re: [PATCH v3 00/16] (few more) Steps towards enabling -Wshadow, Markus Armbruster, 2023/10/06