Re: [PATCH 3/3] system/vl.c: inline qemu_opts_parse_noisily() result checks

2026-04-29 Thread Philippe Mathieu-Daudé 

On 29/4/26 23:03, Richard Henderson wrote:

On 4/29/26 18:04, Philippe Mathieu-Daudé wrote:

On 29/4/26 08:20, Bin Guo wrote:

In qemu_init()'s option parsing switch, several cases assigned the
return value of qemu_opts_parse_noisily() to the shared 'opts'
variable solely to check for NULL, without using the pointer
afterwards.  Inline the call directly into the if-condition, matching
the style already used by QEMU_OPTION_action.

This affects the following options:
   -drive, -numa, -iscsi, -m, -mon, -chardev, -fsdev, -fwcfg

Cases where the returned QemuOpts* is subsequently used (e.g.
-acpitable, -smbios, -virtfs) are left unchanged.

Signed-off-by: Bin Guo 
---
  system/vl.c | 37 +++--
  1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/system/vl.c b/system/vl.c
index 0e1fc217b4..0e0d3cb761 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2961,9 +2961,8 @@ void qemu_init(int argc, char **argv)
  break;
  }
  case QEMU_OPTION_drive:
-    opts = qemu_opts_parse_noisily(qemu_find_opts("drive"),
-   optarg, false);
-    if (opts == NULL) {
+    if (!qemu_opts_parse_noisily(qemu_find_opts("drive"),
+ optarg, false)) {
  exit(1);
  }
  break;


For clarity I'd also squash in this patch:

-- >8 --
diff --git a/system/vl.c b/system/vl.c
index cab8106984a..237334236aa 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2842,3 +2842,2 @@ void qemu_init(int argc, char **argv)
  {
-    QemuOpts *opts;
  QemuOpts *icount_opts = NULL, *accel_opts = NULL;
@@ -2921,2 +2920,4 @@ void qemu_init(int argc, char **argv)
  for(;;) {
+    QemuOpts *opts;
+
  if (optind >= argc)
---

Reviewed-by: Philippe Mathieu-Daudé 


If you're going to move the decl, the innermost block is just below with

     } else {
     const QEMUOption *popt;
+   QemuOpts *opts;

     popt = lookup_opt(argc, argv, &optarg, &optind);


Good catch.

Re: [PATCH 3/3] system/vl.c: inline qemu_opts_parse_noisily() result checks

2026-04-29 Thread Richard Henderson 

On 4/29/26 18:04, Philippe Mathieu-Daudé wrote:

On 29/4/26 08:20, Bin Guo wrote:

In qemu_init()'s option parsing switch, several cases assigned the
return value of qemu_opts_parse_noisily() to the shared 'opts'
variable solely to check for NULL, without using the pointer
afterwards.  Inline the call directly into the if-condition, matching
the style already used by QEMU_OPTION_action.

This affects the following options:
   -drive, -numa, -iscsi, -m, -mon, -chardev, -fsdev, -fwcfg

Cases where the returned QemuOpts* is subsequently used (e.g.
-acpitable, -smbios, -virtfs) are left unchanged.

Signed-off-by: Bin Guo 
---
  system/vl.c | 37 +++--
  1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/system/vl.c b/system/vl.c
index 0e1fc217b4..0e0d3cb761 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2961,9 +2961,8 @@ void qemu_init(int argc, char **argv)
  break;
  }
  case QEMU_OPTION_drive:
-    opts = qemu_opts_parse_noisily(qemu_find_opts("drive"),
-   optarg, false);
-    if (opts == NULL) {
+    if (!qemu_opts_parse_noisily(qemu_find_opts("drive"),
+ optarg, false)) {
  exit(1);
  }
  break;


For clarity I'd also squash in this patch:

-- >8 --
diff --git a/system/vl.c b/system/vl.c
index cab8106984a..237334236aa 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2842,3 +2842,2 @@ void qemu_init(int argc, char **argv)
  {
-    QemuOpts *opts;
  QemuOpts *icount_opts = NULL, *accel_opts = NULL;
@@ -2921,2 +2920,4 @@ void qemu_init(int argc, char **argv)
  for(;;) {
+    QemuOpts *opts;
+
  if (optind >= argc)
---

Reviewed-by: Philippe Mathieu-Daudé 


If you're going to move the decl, the innermost block is just below with

} else {
const QEMUOption *popt;
+   QemuOpts *opts;

popt = lookup_opt(argc, argv, &optarg, &optind);


r~

Re: [PATCH 3/3] system/vl.c: inline qemu_opts_parse_noisily() result checks

2026-04-29 Thread Philippe Mathieu-Daudé 

On 29/4/26 08:20, Bin Guo wrote:

In qemu_init()'s option parsing switch, several cases assigned the
return value of qemu_opts_parse_noisily() to the shared 'opts'
variable solely to check for NULL, without using the pointer
afterwards.  Inline the call directly into the if-condition, matching
the style already used by QEMU_OPTION_action.

This affects the following options:
   -drive, -numa, -iscsi, -m, -mon, -chardev, -fsdev, -fwcfg

Cases where the returned QemuOpts* is subsequently used (e.g.
-acpitable, -smbios, -virtfs) are left unchanged.

Signed-off-by: Bin Guo 
---
  system/vl.c | 37 +++--
  1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/system/vl.c b/system/vl.c
index 0e1fc217b4..0e0d3cb761 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2961,9 +2961,8 @@ void qemu_init(int argc, char **argv)
  break;
  }
  case QEMU_OPTION_drive:
-opts = qemu_opts_parse_noisily(qemu_find_opts("drive"),
-   optarg, false);
-if (opts == NULL) {
+if (!qemu_opts_parse_noisily(qemu_find_opts("drive"),
+ optarg, false)) {
  exit(1);
  }
  break;


For clarity I'd also squash in this patch:

-- >8 --
diff --git a/system/vl.c b/system/vl.c
index cab8106984a..237334236aa 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2842,3 +2842,2 @@ void qemu_init(int argc, char **argv)
 {
-QemuOpts *opts;
 QemuOpts *icount_opts = NULL, *accel_opts = NULL;
@@ -2921,2 +2920,4 @@ void qemu_init(int argc, char **argv)
 for(;;) {
+QemuOpts *opts;
+
 if (optind >= argc)
---

Reviewed-by: Philippe Mathieu-Daudé

[PATCH 3/3] system/vl.c: inline qemu_opts_parse_noisily() result checks

2026-04-28 Thread Bin Guo 
In qemu_init()'s option parsing switch, several cases assigned the
return value of qemu_opts_parse_noisily() to the shared 'opts'
variable solely to check for NULL, without using the pointer
afterwards.  Inline the call directly into the if-condition, matching
the style already used by QEMU_OPTION_action.

This affects the following options:
  -drive, -numa, -iscsi, -m, -mon, -chardev, -fsdev, -fwcfg

Cases where the returned QemuOpts* is subsequently used (e.g.
-acpitable, -smbios, -virtfs) are left unchanged.

Signed-off-by: Bin Guo 
---
 system/vl.c | 37 +++--
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/system/vl.c b/system/vl.c
index 0e1fc217b4..0e0d3cb761 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2961,9 +2961,8 @@ void qemu_init(int argc, char **argv)
 break;
 }
 case QEMU_OPTION_drive:
-opts = qemu_opts_parse_noisily(qemu_find_opts("drive"),
-   optarg, false);
-if (opts == NULL) {
+if (!qemu_opts_parse_noisily(qemu_find_opts("drive"),
+ optarg, false)) {
 exit(1);
 }
 break;
@@ -2988,9 +2987,8 @@ void qemu_init(int argc, char **argv)
 replay_add_blocker("-snapshot");
 break;
 case QEMU_OPTION_numa:
-opts = qemu_opts_parse_noisily(qemu_find_opts("numa"),
-   optarg, true);
-if (!opts) {
+if (!qemu_opts_parse_noisily(qemu_find_opts("numa"),
+ optarg, true)) {
 exit(1);
 }
 break;
@@ -3049,9 +3047,8 @@ void qemu_init(int argc, char **argv)
 break;
 #ifdef CONFIG_LIBISCSI
 case QEMU_OPTION_iscsi:
-opts = qemu_opts_parse_noisily(qemu_find_opts("iscsi"),
-   optarg, false);
-if (!opts) {
+if (!qemu_opts_parse_noisily(qemu_find_opts("iscsi"),
+ optarg, false)) {
 exit(1);
 }
 break;
@@ -3104,8 +3101,8 @@ void qemu_init(int argc, char **argv)
 exit(0);
 break;
 case QEMU_OPTION_m:
-opts = qemu_opts_parse_noisily(qemu_find_opts("memory"), 
optarg, true);
-if (opts == NULL) {
+if (!qemu_opts_parse_noisily(qemu_find_opts("memory"),
+ optarg, true)) {
 exit(1);
 }
 break;
@@ -3226,17 +3223,15 @@ void qemu_init(int argc, char **argv)
 default_monitor = 0;
 break;
 case QEMU_OPTION_mon:
-opts = qemu_opts_parse_noisily(qemu_find_opts("mon"), optarg,
-   true);
-if (!opts) {
+if (!qemu_opts_parse_noisily(qemu_find_opts("mon"), optarg,
+ true)) {
 exit(1);
 }
 default_monitor = 0;
 break;
 case QEMU_OPTION_chardev:
-opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"),
-   optarg, true);
-if (!opts) {
+if (!qemu_opts_parse_noisily(qemu_find_opts("chardev"),
+ optarg, true)) {
 exit(1);
 }
 break;
@@ -3246,8 +3241,7 @@ void qemu_init(int argc, char **argv)
 error_report("fsdev support is disabled");
 exit(1);
 }
-opts = qemu_opts_parse_noisily(olist, optarg, true);
-if (!opts) {
+if (!qemu_opts_parse_noisily(olist, optarg, true)) {
 exit(1);
 }
 break;
@@ -3386,9 +3380,8 @@ void qemu_init(int argc, char **argv)
 smbios_entry_add(opts, &error_fatal);
 break;
 case QEMU_OPTION_fwcfg:
-opts = qemu_opts_parse_noisily(qemu_find_opts("fw_cfg"),
-   optarg, true);
-if (opts == NULL) {
+if (!qemu_opts_parse_noisily(qemu_find_opts("fw_cfg"),
+ optarg, true)) {
 exit(1);
 }
 break;
-- 
2.50.1 (Apple Git-155)