diff mbox series

[RFC,v5,023/126] hw/vfio/ap: drop local_err from vfio_ap_realize

Message ID 20191011160552.22907-24-vsementsov@virtuozzo.com
State New
Headers show
Series error: auto propagated local_err | expand

Commit Message

Vladimir Sementsov-Ogievskiy Oct. 11, 2019, 4:04 p.m. UTC 
No reason for local_err here, use errp directly instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 hw/vfio/ap.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

Comments

Marc-André Lureau Nov. 8, 2019, 9 p.m. UTC | #1 
On Fri, Oct 11, 2019 at 9:27 PM Vladimir Sementsov-Ogievskiy
<vsementsov@virtuozzo.com> wrote:
>
> No reason for local_err here, use errp directly instead.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  hw/vfio/ap.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index da6a20669d..db816e1860 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -87,16 +87,14 @@ static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
>
>  static void vfio_ap_realize(DeviceState *dev, Error **errp)
>  {
> -    int ret;
>      char *mdevid;
> -    Error *local_err = NULL;
>      VFIOGroup *vfio_group;
>      APDevice *apdev = AP_DEVICE(dev);
>      VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
>
> -    vfio_group = vfio_ap_get_group(vapdev, &local_err);
> +    vfio_group = vfio_ap_get_group(vapdev, errp);
>      if (!vfio_group) {
> -        goto out_err;
> +        return;
>      }
>
>      vapdev->vdev.ops = &vfio_ap_ops;
> @@ -113,18 +111,10 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
>       */
>      vapdev->vdev.balloon_allowed = true;
>
> -    ret = vfio_get_device(vfio_group, mdevid, &vapdev->vdev, &local_err);
> -    if (ret) {
> -        goto out_get_dev_err;
> -    }
> -
> -    return;
> +    vfio_get_device(vfio_group, mdevid, &vapdev->vdev, errp);
>
> -out_get_dev_err:
>      vfio_ap_put_device(vapdev);
>      vfio_put_group(vfio_group);
> -out_err:
> -    error_propagate(errp, local_err);
>  }
>
>  static void vfio_ap_unrealize(DeviceState *dev, Error **errp)
> --
> 2.21.0
>
>


Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Cornelia Huck Nov. 12, 2019, 1:06 p.m. UTC | #2 
On Fri, 11 Oct 2019 19:04:09 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> No reason for local_err here, use errp directly instead.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  hw/vfio/ap.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index da6a20669d..db816e1860 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -87,16 +87,14 @@ static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
>  
>  static void vfio_ap_realize(DeviceState *dev, Error **errp)
>  {
> -    int ret;
>      char *mdevid;
> -    Error *local_err = NULL;
>      VFIOGroup *vfio_group;
>      APDevice *apdev = AP_DEVICE(dev);
>      VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
>  
> -    vfio_group = vfio_ap_get_group(vapdev, &local_err);
> +    vfio_group = vfio_ap_get_group(vapdev, errp);
>      if (!vfio_group) {
> -        goto out_err;
> +        return;
>      }
>  
>      vapdev->vdev.ops = &vfio_ap_ops;
> @@ -113,18 +111,10 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
>       */
>      vapdev->vdev.balloon_allowed = true;
>  
> -    ret = vfio_get_device(vfio_group, mdevid, &vapdev->vdev, &local_err);
> -    if (ret) {
> -        goto out_get_dev_err;
> -    }
> -
> -    return;
> +    vfio_get_device(vfio_group, mdevid, &vapdev->vdev, errp);

This looks wrong; you still need to check for the outcome of
vfio_get_device().

>  
> -out_get_dev_err:
>      vfio_ap_put_device(vapdev);
>      vfio_put_group(vfio_group);
> -out_err:
> -    error_propagate(errp, local_err);
>  }
>  
>  static void vfio_ap_unrealize(DeviceState *dev, Error **errp)
Vladimir Sementsov-Ogievskiy Nov. 12, 2019, 3:29 p.m. UTC | #3 
12.11.2019 16:06, Cornelia Huck wrote:
> On Fri, 11 Oct 2019 19:04:09 +0300
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
> 
>> No reason for local_err here, use errp directly instead.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   hw/vfio/ap.c | 16 +++-------------
>>   1 file changed, 3 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
>> index da6a20669d..db816e1860 100644
>> --- a/hw/vfio/ap.c
>> +++ b/hw/vfio/ap.c
>> @@ -87,16 +87,14 @@ static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
>>   
>>   static void vfio_ap_realize(DeviceState *dev, Error **errp)
>>   {
>> -    int ret;
>>       char *mdevid;
>> -    Error *local_err = NULL;
>>       VFIOGroup *vfio_group;
>>       APDevice *apdev = AP_DEVICE(dev);
>>       VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
>>   
>> -    vfio_group = vfio_ap_get_group(vapdev, &local_err);
>> +    vfio_group = vfio_ap_get_group(vapdev, errp);
>>       if (!vfio_group) {
>> -        goto out_err;
>> +        return;
>>       }
>>   
>>       vapdev->vdev.ops = &vfio_ap_ops;
>> @@ -113,18 +111,10 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
>>        */
>>       vapdev->vdev.balloon_allowed = true;
>>   
>> -    ret = vfio_get_device(vfio_group, mdevid, &vapdev->vdev, &local_err);
>> -    if (ret) {
>> -        goto out_get_dev_err;
>> -    }
>> -
>> -    return;
>> +    vfio_get_device(vfio_group, mdevid, &vapdev->vdev, errp);
> 
> This looks wrong; you still need to check for the outcome of
> vfio_get_device().

Oops, agree, will fix.


> 
>>   
>> -out_get_dev_err:
>>       vfio_ap_put_device(vapdev);
>>       vfio_put_group(vfio_group);
>> -out_err:
>> -    error_propagate(errp, local_err);
>>   }
>>   
>>   static void vfio_ap_unrealize(DeviceState *dev, Error **errp)
>
diff mbox series

Patch

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index da6a20669d..db816e1860 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -87,16 +87,14 @@  static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
 
 static void vfio_ap_realize(DeviceState *dev, Error **errp)
 {
-    int ret;
     char *mdevid;
-    Error *local_err = NULL;
     VFIOGroup *vfio_group;
     APDevice *apdev = AP_DEVICE(dev);
     VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
 
-    vfio_group = vfio_ap_get_group(vapdev, &local_err);
+    vfio_group = vfio_ap_get_group(vapdev, errp);
     if (!vfio_group) {
-        goto out_err;
+        return;
     }
 
     vapdev->vdev.ops = &vfio_ap_ops;
@@ -113,18 +111,10 @@  static void vfio_ap_realize(DeviceState *dev, Error **errp)
      */
     vapdev->vdev.balloon_allowed = true;
 
-    ret = vfio_get_device(vfio_group, mdevid, &vapdev->vdev, &local_err);
-    if (ret) {
-        goto out_get_dev_err;
-    }
-
-    return;
+    vfio_get_device(vfio_group, mdevid, &vapdev->vdev, errp);
 
-out_get_dev_err:
     vfio_ap_put_device(vapdev);
     vfio_put_group(vfio_group);
-out_err:
-    error_propagate(errp, local_err);
 }
 
 static void vfio_ap_unrealize(DeviceState *dev, Error **errp)