diff mbox series

[02/16] vfio/display: Make vfio_display_*() return bool

Message ID 20240515082041.556571-3-zhenzhong.duan@intel.com (mailing list archive)
State New, archived
Headers show
Series VFIO: misc cleanups part2 | expand

Commit Message

Duan, Zhenzhong May 15, 2024, 8:20 a.m. UTC 
This is to follow the coding standand in qapi/error.h to return bool
for bool-valued functions.

Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/pci.h     |  2 +-
 hw/vfio/display.c | 20 ++++++++++----------
 hw/vfio/pci.c     |  3 +--
 3 files changed, 12 insertions(+), 13 deletions(-)

Comments

Cédric Le Goater May 21, 2024, 12:14 p.m. UTC | #1 
On 5/15/24 10:20, Zhenzhong Duan wrote:
> This is to follow the coding standand in qapi/error.h to return bool
> for bool-valued functions.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

One comment below,

> ---
>   hw/vfio/pci.h     |  2 +-
>   hw/vfio/display.c | 20 ++++++++++----------
>   hw/vfio/pci.c     |  3 +--
>   3 files changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index 92cd62d115..a5ac9efd4b 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -232,7 +232,7 @@ int vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
>                                  Error **errp);
>   
>   void vfio_display_reset(VFIOPCIDevice *vdev);
> -int vfio_display_probe(VFIOPCIDevice *vdev, Error **errp);
> +bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp);
>   void vfio_display_finalize(VFIOPCIDevice *vdev);
>   
>   extern const VMStateDescription vfio_display_vmstate;
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index 57c5ae0b2a..b562f4be74 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -346,11 +346,11 @@ static const GraphicHwOps vfio_display_dmabuf_ops = {
>       .ui_info    = vfio_display_edid_ui_info,
>   };
>   
> -static int vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
> +static bool vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
>   {
>       if (!display_opengl) {
>           error_setg(errp, "vfio-display-dmabuf: opengl not available");
> -        return -1;
> +        return false;
>       }
>   
>       vdev->dpy = g_new0(VFIODisplay, 1);
> @@ -360,11 +360,11 @@ static int vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
>       if (vdev->enable_ramfb) {
>           vdev->dpy->ramfb = ramfb_setup(errp);
>           if (!vdev->dpy->ramfb) {
> -            return -EINVAL;
> +            return false;
>           }
>       }
>       vfio_display_edid_init(vdev);

vfio_display_edid_init() can fail for many reasons and does it silently.
It would be good to report the error in a future patch.

Thanks,

C.



> -    return 0;
> +    return true;
>   }
>   
>   static void vfio_display_dmabuf_exit(VFIODisplay *dpy)
> @@ -481,7 +481,7 @@ static const GraphicHwOps vfio_display_region_ops = {
>       .gfx_update = vfio_display_region_update,
>   };
>   
> -static int vfio_display_region_init(VFIOPCIDevice *vdev, Error **errp)
> +static bool vfio_display_region_init(VFIOPCIDevice *vdev, Error **errp)
>   {
>       vdev->dpy = g_new0(VFIODisplay, 1);
>       vdev->dpy->con = graphic_console_init(DEVICE(vdev), 0,
> @@ -490,10 +490,10 @@ static int vfio_display_region_init(VFIOPCIDevice *vdev, Error **errp)
>       if (vdev->enable_ramfb) {
>           vdev->dpy->ramfb = ramfb_setup(errp);
>           if (!vdev->dpy->ramfb) {
> -            return -EINVAL;
> +            return false;
>           }
>       }
> -    return 0;
> +    return true;
>   }
>   
>   static void vfio_display_region_exit(VFIODisplay *dpy)
> @@ -508,7 +508,7 @@ static void vfio_display_region_exit(VFIODisplay *dpy)
>   
>   /* ---------------------------------------------------------------------- */
>   
> -int vfio_display_probe(VFIOPCIDevice *vdev, Error **errp)
> +bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp)
>   {
>       struct vfio_device_gfx_plane_info probe;
>       int ret;
> @@ -531,11 +531,11 @@ int vfio_display_probe(VFIOPCIDevice *vdev, Error **errp)
>   
>       if (vdev->display == ON_OFF_AUTO_AUTO) {
>           /* not an error in automatic mode */
> -        return 0;
> +        return true;
>       }
>   
>       error_setg(errp, "vfio: device doesn't support any (known) display method");
> -    return -1;
> +    return false;
>   }
>   
>   void vfio_display_finalize(VFIOPCIDevice *vdev)
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index c1adef5cf8..a447013a1d 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3200,8 +3200,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>       }
>   
>       if (vdev->display != ON_OFF_AUTO_OFF) {
> -        ret = vfio_display_probe(vdev, errp);
> -        if (ret) {
> +        if (!vfio_display_probe(vdev, errp)) {
>               goto out_deregister;
>           }
>       }
Duan, Zhenzhong May 22, 2024, 4:45 a.m. UTC | #2 
>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH 02/16] vfio/display: Make vfio_display_*() return bool
>
>On 5/15/24 10:20, Zhenzhong Duan wrote:
>> This is to follow the coding standand in qapi/error.h to return bool
>> for bool-valued functions.
>>
>> Suggested-by: Cédric Le Goater <clg@redhat.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>
>
>Reviewed-by: Cédric Le Goater <clg@redhat.com>
>
>One comment below,
>
>> ---
>>   hw/vfio/pci.h     |  2 +-
>>   hw/vfio/display.c | 20 ++++++++++----------
>>   hw/vfio/pci.c     |  3 +--
>>   3 files changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
>> index 92cd62d115..a5ac9efd4b 100644
>> --- a/hw/vfio/pci.h
>> +++ b/hw/vfio/pci.h
>> @@ -232,7 +232,7 @@ int vfio_pci_igd_opregion_init(VFIOPCIDevice
>*vdev,
>>                                  Error **errp);
>>
>>   void vfio_display_reset(VFIOPCIDevice *vdev);
>> -int vfio_display_probe(VFIOPCIDevice *vdev, Error **errp);
>> +bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp);
>>   void vfio_display_finalize(VFIOPCIDevice *vdev);
>>
>>   extern const VMStateDescription vfio_display_vmstate;
>> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
>> index 57c5ae0b2a..b562f4be74 100644
>> --- a/hw/vfio/display.c
>> +++ b/hw/vfio/display.c
>> @@ -346,11 +346,11 @@ static const GraphicHwOps
>vfio_display_dmabuf_ops = {
>>       .ui_info    = vfio_display_edid_ui_info,
>>   };
>>
>> -static int vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
>> +static bool vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
>>   {
>>       if (!display_opengl) {
>>           error_setg(errp, "vfio-display-dmabuf: opengl not available");
>> -        return -1;
>> +        return false;
>>       }
>>
>>       vdev->dpy = g_new0(VFIODisplay, 1);
>> @@ -360,11 +360,11 @@ static int
>vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
>>       if (vdev->enable_ramfb) {
>>           vdev->dpy->ramfb = ramfb_setup(errp);
>>           if (!vdev->dpy->ramfb) {
>> -            return -EINVAL;
>> +            return false;
>>           }
>>       }
>>       vfio_display_edid_init(vdev);
>
>vfio_display_edid_init() can fail for many reasons and does it silently.
>It would be good to report the error in a future patch.

Yes, that deserve a fix. Will address it with a future patch.

Thanks
Zhenzhong
diff mbox series

Patch

diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 92cd62d115..a5ac9efd4b 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -232,7 +232,7 @@  int vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
                                Error **errp);
 
 void vfio_display_reset(VFIOPCIDevice *vdev);
-int vfio_display_probe(VFIOPCIDevice *vdev, Error **errp);
+bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp);
 void vfio_display_finalize(VFIOPCIDevice *vdev);
 
 extern const VMStateDescription vfio_display_vmstate;
diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index 57c5ae0b2a..b562f4be74 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -346,11 +346,11 @@  static const GraphicHwOps vfio_display_dmabuf_ops = {
     .ui_info    = vfio_display_edid_ui_info,
 };
 
-static int vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
+static bool vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
 {
     if (!display_opengl) {
         error_setg(errp, "vfio-display-dmabuf: opengl not available");
-        return -1;
+        return false;
     }
 
     vdev->dpy = g_new0(VFIODisplay, 1);
@@ -360,11 +360,11 @@  static int vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
     if (vdev->enable_ramfb) {
         vdev->dpy->ramfb = ramfb_setup(errp);
         if (!vdev->dpy->ramfb) {
-            return -EINVAL;
+            return false;
         }
     }
     vfio_display_edid_init(vdev);
-    return 0;
+    return true;
 }
 
 static void vfio_display_dmabuf_exit(VFIODisplay *dpy)
@@ -481,7 +481,7 @@  static const GraphicHwOps vfio_display_region_ops = {
     .gfx_update = vfio_display_region_update,
 };
 
-static int vfio_display_region_init(VFIOPCIDevice *vdev, Error **errp)
+static bool vfio_display_region_init(VFIOPCIDevice *vdev, Error **errp)
 {
     vdev->dpy = g_new0(VFIODisplay, 1);
     vdev->dpy->con = graphic_console_init(DEVICE(vdev), 0,
@@ -490,10 +490,10 @@  static int vfio_display_region_init(VFIOPCIDevice *vdev, Error **errp)
     if (vdev->enable_ramfb) {
         vdev->dpy->ramfb = ramfb_setup(errp);
         if (!vdev->dpy->ramfb) {
-            return -EINVAL;
+            return false;
         }
     }
-    return 0;
+    return true;
 }
 
 static void vfio_display_region_exit(VFIODisplay *dpy)
@@ -508,7 +508,7 @@  static void vfio_display_region_exit(VFIODisplay *dpy)
 
 /* ---------------------------------------------------------------------- */
 
-int vfio_display_probe(VFIOPCIDevice *vdev, Error **errp)
+bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp)
 {
     struct vfio_device_gfx_plane_info probe;
     int ret;
@@ -531,11 +531,11 @@  int vfio_display_probe(VFIOPCIDevice *vdev, Error **errp)
 
     if (vdev->display == ON_OFF_AUTO_AUTO) {
         /* not an error in automatic mode */
-        return 0;
+        return true;
     }
 
     error_setg(errp, "vfio: device doesn't support any (known) display method");
-    return -1;
+    return false;
 }
 
 void vfio_display_finalize(VFIOPCIDevice *vdev)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index c1adef5cf8..a447013a1d 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3200,8 +3200,7 @@  static void vfio_realize(PCIDevice *pdev, Error **errp)
     }
 
     if (vdev->display != ON_OFF_AUTO_OFF) {
-        ret = vfio_display_probe(vdev, errp);
-        if (ret) {
+        if (!vfio_display_probe(vdev, errp)) {
             goto out_deregister;
         }
     }