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é