Re: [PATCH 17/46] qemu-option: Smooth error checking with Coccinelle

[Eric Blake]

On 6/24/20 11:43 AM, Markus Armbruster wrote:
The previous commit enables conversion of
     foo(..., &err);
     if (err) {
         ...
     }

to

     if (!foo(..., &err)) {
         ...
     }

for QemuOpts functions that now return true / false on success /
error.  Coccinelle script:



Eliminate error_propagate() that are now unnecessary.  Delete @err
that are now unused.  Tidy up line breaks and whitespace.

Signed-off-by: Markus Armbruster <arm...@redhat.com>
---

  32 files changed, 70 insertions(+), 192 deletions(-)

Another decent chunk of cleanups.


diff --git a/block.c b/block.c
index 30a72bc4c2..77e85f13db 100644
--- a/block.c
+++ b/block.c

@@ -6091,8 +6086,8 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
      }
  
      if (base_filename) {
-        qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename, &local_err);
-        if (local_err) {
+        if (!qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename,
+                          &local_err)) {
              error_setg(errp, "Backing file not supported for file format 
'%s'",
                         fmt);

Pre-existing - it is odd that we collect a message into local_err, then 
write something else into errp; the out: label does 
error_propagate(errp, local_err) which ensures there is no leak but 
discards the original err.  You could pass NULL instead.  But as it is 
pre-existing, passing NULL should be a separate patch.

              goto out;
@@ -6100,8 +6095,7 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
      }
  
      if (base_fmt) {
-        qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &local_err);
-        if (local_err) {
+        if (!qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &local_err)) {
              error_setg(errp, "Backing file format not supported for file "
                               "format '%s'", fmt);

Ditto.

+++ b/qemu-img.c
@@ -467,8 +467,8 @@ static int add_old_style_options(const char *fmt, QemuOpts 
*opts,
      Error *err = NULL;
  
      if (base_filename) {
-        qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename, &err);
-        if (err) {
+        if (!qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename,
+                          &err)) {
              error_report("Backing file not supported for file format '%s'",
                           fmt);
              error_free(err);
@@ -476,8 +476,7 @@ static int add_old_style_options(const char *fmt, QemuOpts 
*opts,
          }
      }
      if (base_fmt) {
-        qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &err);
-        if (err) {
+        if (!qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &err)) {
              error_report("Backing file format not supported for file "
                           "format '%s'", fmt);
              error_free(err);

Ditto.

But the conversion here is sane.
Reviewed-by: Eric Blake <ebl...@redhat.com>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org