Re: [PATCH 3/3] xen-block: Use one Error * variable instead of two

[Markus Armbruster]

Uh, I failed to actually send this.  It's in my pull request now.

Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes:

> 13.03.2020 20:05, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>>   hw/block/xen-block.c | 5 +----
>>   1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
>> index 3885464513..7b3b6dee97 100644
>> --- a/hw/block/xen-block.c
>> +++ b/hw/block/xen-block.c
>> @@ -998,14 +998,13 @@ static void 
>> xen_block_device_destroy(XenBackendInstance *backend,
>>       XenBlockVdev *vdev = &blockdev->props.vdev;
>>       XenBlockDrive *drive = blockdev->drive;
>>       XenBlockIOThread *iothread = blockdev->iothread;
>> +    Error *local_err = NULL;
>>         trace_xen_block_device_destroy(vdev->number);
>>         object_unparent(OBJECT(xendev));
>>         if (iothread) {
>> -        Error *local_err = NULL;
>> -
>>           xen_block_iothread_destroy(iothread, &local_err);
>>           if (local_err) {
>>               error_propagate_prepend(errp, local_err,
>> @@ -1015,8 +1014,6 @@ static void 
>> xen_block_device_destroy(XenBackendInstance *backend,
>>       }
>>         if (drive) {
>> -        Error *local_err = NULL;
>> -
>>           xen_block_drive_destroy(drive, &local_err);
>>           if (local_err) {
>>               error_propagate_prepend(errp, local_err,
>
> Hmm, no "return;" statement after this propagation. It's OK, as there no more 
> code in the function after this "if", but I'd add it to be consistent and to 
> avoid forgetting to add a return here when add more code to the function.
>
> (and if you do this, you may also fix indentation of string paramter of 
> error_propagate_prepend...)
>
>
>
> Anyway,
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>

Like this, I guess:

xen-block: Use one Error * variable instead of two

While there, tidy up indentation, and add return just for consistency
and robustness.

Signed-off-by: Markus Armbruster <arm...@redhat.com>
Message-Id: <20200313170517.22480-4-arm...@redhat.com>
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
Reviewed-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>

diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 3885464513..07bb32e22b 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -998,29 +998,27 @@ static void xen_block_device_destroy(XenBackendInstance 
*backend,
     XenBlockVdev *vdev = &blockdev->props.vdev;
     XenBlockDrive *drive = blockdev->drive;
     XenBlockIOThread *iothread = blockdev->iothread;
+    Error *local_err = NULL;
 
     trace_xen_block_device_destroy(vdev->number);
 
     object_unparent(OBJECT(xendev));
 
     if (iothread) {
-        Error *local_err = NULL;
-
         xen_block_iothread_destroy(iothread, &local_err);
         if (local_err) {
             error_propagate_prepend(errp, local_err,
-                                "failed to destroy iothread: ");
+                                    "failed to destroy iothread: ");
             return;
         }
     }
 
     if (drive) {
-        Error *local_err = NULL;
-
         xen_block_drive_destroy(drive, &local_err);
         if (local_err) {
             error_propagate_prepend(errp, local_err,
-                                "failed to destroy drive: ");
+                                    "failed to destroy drive: ");
+            return;
         }
     }
 }