error: Mechanical fixes & cleanups

[PATCH 3/4] error: Remove NULL checks on error_propagate() calls (again)

Posted by Markus Armbruster 5 years, 3 months ago

Patch created mechanically by rerunning:

    $ spatch --sp-file scripts/coccinelle/error_propagate_null.cocci \
             --macro-file scripts/cocci-macro-file.h \
             --use-gitgrep .

Cc: Jens Freimann <jfreimann@redhat.com>
Cc: Hailiang Zhang <zhang.zhanghailiang@huawei.com>
Cc: Juan Quintela <quintela@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/net/virtio-net.c   | 8 ++------
 migration/colo.c      | 4 +---
 migration/migration.c | 8 ++------
 3 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 4895af1cbe..21b9a315ed 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -809,9 +809,7 @@ static void failover_add_primary(VirtIONet *n, Error **errp)
             "sure primary device has parameter"
             " failover_pair_id=<virtio-net-id>\n");
 }
-    if (err) {
-        error_propagate(errp, err);
-    }
+    error_propagate(errp, err);
 }
 
 static int is_my_primary(void *opaque, QemuOpts *opts, Error **errp)
@@ -865,9 +863,7 @@ static DeviceState *virtio_connect_failover_devices(VirtIONet *n,
         n->primary_device_id = g_strdup(prim_dev->id);
         n->primary_device_opts = prim_dev->opts;
     } else {
-        if (err) {
-            error_propagate(errp, err);
-        }
+        error_propagate(errp, err);
     }
 
     return prim_dev;
diff --git a/migration/colo.c b/migration/colo.c
index ea7d1e9d4e..c6848c829b 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -798,9 +798,7 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
 
     colo_send_message(mis->to_src_file, COLO_MESSAGE_VMSTATE_LOADED,
                  &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-    }
+    error_propagate(errp, local_err);
 }
 
 static void colo_wait_handle_message(MigrationIncomingState *mis,
diff --git a/migration/migration.c b/migration/migration.c
index 2ed9923227..6a458b8fe5 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -601,9 +601,7 @@ void migration_fd_process_incoming(QEMUFile *f, Error **errp)
     }
 
     if (migration_incoming_setup(f, &local_err)) {
-        if (local_err) {
-            error_propagate(errp, local_err);
-        }
+        error_propagate(errp, local_err);
         return;
     }
     migration_incoming_process();
@@ -625,9 +623,7 @@ void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp)
         }
 
         if (migration_incoming_setup(f, &local_err)) {
-            if (local_err) {
-                error_propagate(errp, local_err);
-            }
+            error_propagate(errp, local_err);
             return;
         }
 
-- 
2.26.2

Re: [PATCH 3/4] error: Remove NULL checks on error_propagate() calls (again)

Posted by Eric Blake 5 years, 3 months ago

On 7/22/20 3:40 AM, Markus Armbruster wrote:
> Patch created mechanically by rerunning:
> 
>      $ spatch --sp-file scripts/coccinelle/error_propagate_null.cocci \
>               --macro-file scripts/cocci-macro-file.h \
>               --use-gitgrep .
> 
> Cc: Jens Freimann <jfreimann@redhat.com>
> Cc: Hailiang Zhang <zhang.zhanghailiang@huawei.com>
> Cc: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

> +++ b/migration/colo.c
> @@ -798,9 +798,7 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
>   
>       colo_send_message(mis->to_src_file, COLO_MESSAGE_VMSTATE_LOADED,
>                    &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -    }
> +    error_propagate(errp, local_err);
>   }

As this is mechanical, it is fine. But there is now a further cleanup 
possible of passing errp directly to colo_send_message, and possibly 
dropping local_err altogether.

Reviewed-by: Eric Blake <eblake@redhat.com>

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

Re: [PATCH 3/4] error: Remove NULL checks on error_propagate() calls (again)

Posted by Markus Armbruster 5 years, 3 months ago

Eric Blake <eblake@redhat.com> writes:

> On 7/22/20 3:40 AM, Markus Armbruster wrote:
>> Patch created mechanically by rerunning:
>>
>>      $ spatch --sp-file scripts/coccinelle/error_propagate_null.cocci \
>>               --macro-file scripts/cocci-macro-file.h \
>>               --use-gitgrep .
>>
>> Cc: Jens Freimann <jfreimann@redhat.com>
>> Cc: Hailiang Zhang <zhang.zhanghailiang@huawei.com>
>> Cc: Juan Quintela <quintela@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
>> +++ b/migration/colo.c
>> @@ -798,9 +798,7 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
>>         colo_send_message(mis->to_src_file,
>> COLO_MESSAGE_VMSTATE_LOADED,
>>                    &local_err);
>> -    if (local_err) {
>> -        error_propagate(errp, local_err);
>> -    }
>> +    error_propagate(errp, local_err);
>>   }
>
> As this is mechanical, it is fine. But there is now a further cleanup
> possible of passing errp directly to colo_send_message, and possibly
> dropping local_err altogether.

True.

The patch is small and simple enough for squashing in further manual
cleanups.  I'd like to first check whether a followup patch created with
the machinery I used for eliminating error_propagate() comes out better.

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

Re: [PATCH 3/4] error: Remove NULL checks on error_propagate() calls (again)

Posted by Markus Armbruster 5 years, 3 months ago

Markus Armbruster <armbru@redhat.com> writes:

> Eric Blake <eblake@redhat.com> writes:
>
>> On 7/22/20 3:40 AM, Markus Armbruster wrote:
>>> Patch created mechanically by rerunning:
>>>
>>>      $ spatch --sp-file scripts/coccinelle/error_propagate_null.cocci \
>>>               --macro-file scripts/cocci-macro-file.h \
>>>               --use-gitgrep .
>>>
>>> Cc: Jens Freimann <jfreimann@redhat.com>
>>> Cc: Hailiang Zhang <zhang.zhanghailiang@huawei.com>
>>> Cc: Juan Quintela <quintela@redhat.com>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>
>>> +++ b/migration/colo.c
>>> @@ -798,9 +798,7 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
>>>         colo_send_message(mis->to_src_file,
>>> COLO_MESSAGE_VMSTATE_LOADED,
>>>                    &local_err);
>>> -    if (local_err) {
>>> -        error_propagate(errp, local_err);
>>> -    }
>>> +    error_propagate(errp, local_err);
>>>   }
>>
>> As this is mechanical, it is fine. But there is now a further cleanup
>> possible of passing errp directly to colo_send_message, and possibly
>> dropping local_err altogether.
>
> True.
>
> The patch is small and simple enough for squashing in further manual
> cleanups.  I'd like to first check whether a followup patch created with
> the machinery I used for eliminating error_propagate() comes out better.

Vladimir's scripts/coccinelle/errp-guard.cocci will take care of it.

Eliminating error propagation altogether would be even better, but it's
also more work: several void functions need to return bool instead.

>> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> Thanks!

Re: [PATCH 3/4] error: Remove NULL checks on error_propagate() calls (again)

Posted by Eric Blake 5 years, 3 months ago

On 7/23/20 8:38 AM, Markus Armbruster wrote:

>>>> +++ b/migration/colo.c
>>>> @@ -798,9 +798,7 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
>>>>          colo_send_message(mis->to_src_file,
>>>> COLO_MESSAGE_VMSTATE_LOADED,
>>>>                     &local_err);
>>>> -    if (local_err) {
>>>> -        error_propagate(errp, local_err);
>>>> -    }
>>>> +    error_propagate(errp, local_err);
>>>>    }
>>>
>>> As this is mechanical, it is fine. But there is now a further cleanup
>>> possible of passing errp directly to colo_send_message, and possibly
>>> dropping local_err altogether.
>>
>> True.
>>
>> The patch is small and simple enough for squashing in further manual
>> cleanups.  I'd like to first check whether a followup patch created with
>> the machinery I used for eliminating error_propagate() comes out better.
> 
> Vladimir's scripts/coccinelle/errp-guard.cocci will take care of it.

Good to know.  Then I'm fine deferring those cleanups to the mechanical 
patches down the road, rather than a manual effort now.

> 
> Eliminating error propagation altogether would be even better, but it's
> also more work: several void functions need to return bool instead.

Correct, but also doesn't change the fact that this patch is ready to go 
regardless of how much further cleanup we plan on doing.

> 
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>
>> Thanks!

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