s390x/ccw: Error reporting cleanups

[PATCH 2/7] s390x/css: Make CCWDeviceClass::realize return bool

Posted by Cédric Le Goater 6 months ago

Since the realize() handler of CCWDeviceClass takes an 'Error **'
argument, best practices suggest to return a bool. See the api/error.h
Rules section. While at it, modify the call in s390_ccw_realize().

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/s390x/ccw-device.h | 2 +-
 hw/s390x/ccw-device.c | 3 ++-
 hw/s390x/s390-ccw.c   | 3 +--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h
index 6dff95225df11c63f9b66975019026b215c8c448..5feeb0ee7a268b8709043b5bbc56b06e707a448d 100644
--- a/hw/s390x/ccw-device.h
+++ b/hw/s390x/ccw-device.h
@@ -36,7 +36,7 @@ extern const VMStateDescription vmstate_ccw_dev;
 struct CCWDeviceClass {
     DeviceClass parent_class;
     void (*unplug)(HotplugHandler *, DeviceState *, Error **);
-    void (*realize)(CcwDevice *, Error **);
+    bool (*realize)(CcwDevice *, Error **);
     void (*refill_ids)(CcwDevice *);
 };
 
diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c
index fb8c1acc64d5002c861a4913f292d8346dbef192..a7d682e5af9ce90e7e2fad8c24b30e39328c7cf4 100644
--- a/hw/s390x/ccw-device.c
+++ b/hw/s390x/ccw-device.c
@@ -31,9 +31,10 @@ static void ccw_device_refill_ids(CcwDevice *dev)
     dev->subch_id.valid = true;
 }
 
-static void ccw_device_realize(CcwDevice *dev, Error **errp)
+static bool ccw_device_realize(CcwDevice *dev, Error **errp)
 {
     ccw_device_refill_ids(dev);
+    return true;
 }
 
 static Property ccw_device_properties[] = {
diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
index a06e91dfb318e3500324851488c56806fa46c08d..4b8ede701df90949720262b6fc1b65f4e505e34d 100644
--- a/hw/s390x/s390-ccw.c
+++ b/hw/s390x/s390-ccw.c
@@ -137,8 +137,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp)
         goto out_err;
     }
 
-    ck->realize(ccw_dev, &err);
-    if (err) {
+    if (!ck->realize(ccw_dev, &err)) {
         goto out_err;
     }
 
-- 
2.45.1

Re: [PATCH 2/7] s390x/css: Make CCWDeviceClass::realize return bool

Posted by Anthony Krowiak 6 months ago

On 5/22/24 1:01 PM, Cédric Le Goater wrote:
> Since the realize() handler of CCWDeviceClass takes an 'Error **'
> argument, best practices suggest to return a bool. See the api/error.h
> Rules section. While at it, modify the call in s390_ccw_realize().
>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>


Reviewed-by: Anthony Krowiak <akrowiak@linux.ibm.com>


> ---
>   hw/s390x/ccw-device.h | 2 +-
>   hw/s390x/ccw-device.c | 3 ++-
>   hw/s390x/s390-ccw.c   | 3 +--
>   3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h
> index 6dff95225df11c63f9b66975019026b215c8c448..5feeb0ee7a268b8709043b5bbc56b06e707a448d 100644
> --- a/hw/s390x/ccw-device.h
> +++ b/hw/s390x/ccw-device.h
> @@ -36,7 +36,7 @@ extern const VMStateDescription vmstate_ccw_dev;
>   struct CCWDeviceClass {
>       DeviceClass parent_class;
>       void (*unplug)(HotplugHandler *, DeviceState *, Error **);
> -    void (*realize)(CcwDevice *, Error **);
> +    bool (*realize)(CcwDevice *, Error **);
>       void (*refill_ids)(CcwDevice *);
>   };
>   
> diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c
> index fb8c1acc64d5002c861a4913f292d8346dbef192..a7d682e5af9ce90e7e2fad8c24b30e39328c7cf4 100644
> --- a/hw/s390x/ccw-device.c
> +++ b/hw/s390x/ccw-device.c
> @@ -31,9 +31,10 @@ static void ccw_device_refill_ids(CcwDevice *dev)
>       dev->subch_id.valid = true;
>   }
>   
> -static void ccw_device_realize(CcwDevice *dev, Error **errp)
> +static bool ccw_device_realize(CcwDevice *dev, Error **errp)
>   {
>       ccw_device_refill_ids(dev);
> +    return true;
>   }
>   
>   static Property ccw_device_properties[] = {
> diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
> index a06e91dfb318e3500324851488c56806fa46c08d..4b8ede701df90949720262b6fc1b65f4e505e34d 100644
> --- a/hw/s390x/s390-ccw.c
> +++ b/hw/s390x/s390-ccw.c
> @@ -137,8 +137,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp)
>           goto out_err;
>       }
>   
> -    ck->realize(ccw_dev, &err);
> -    if (err) {
> +    if (!ck->realize(ccw_dev, &err)) {
>           goto out_err;
>       }
>

RE: [PATCH 2/7] s390x/css: Make CCWDeviceClass::realize return bool

Posted by Duan, Zhenzhong 6 months ago


>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: [PATCH 2/7] s390x/css: Make CCWDeviceClass::realize return bool
>
>Since the realize() handler of CCWDeviceClass takes an 'Error **'
>argument, best practices suggest to return a bool. See the api/error.h
>Rules section. While at it, modify the call in s390_ccw_realize().
>
>Signed-off-by: Cédric Le Goater <clg@redhat.com>

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

>---
> hw/s390x/ccw-device.h | 2 +-
> hw/s390x/ccw-device.c | 3 ++-
> hw/s390x/s390-ccw.c   | 3 +--
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h
>index
>6dff95225df11c63f9b66975019026b215c8c448..5feeb0ee7a268b8709043b
>5bbc56b06e707a448d 100644
>--- a/hw/s390x/ccw-device.h
>+++ b/hw/s390x/ccw-device.h
>@@ -36,7 +36,7 @@ extern const VMStateDescription vmstate_ccw_dev;
> struct CCWDeviceClass {
>     DeviceClass parent_class;
>     void (*unplug)(HotplugHandler *, DeviceState *, Error **);
>-    void (*realize)(CcwDevice *, Error **);
>+    bool (*realize)(CcwDevice *, Error **);
>     void (*refill_ids)(CcwDevice *);
> };
>
>diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c
>index
>fb8c1acc64d5002c861a4913f292d8346dbef192..a7d682e5af9ce90e7e2fad8
>c24b30e39328c7cf4 100644
>--- a/hw/s390x/ccw-device.c
>+++ b/hw/s390x/ccw-device.c
>@@ -31,9 +31,10 @@ static void ccw_device_refill_ids(CcwDevice *dev)
>     dev->subch_id.valid = true;
> }
>
>-static void ccw_device_realize(CcwDevice *dev, Error **errp)
>+static bool ccw_device_realize(CcwDevice *dev, Error **errp)
> {
>     ccw_device_refill_ids(dev);
>+    return true;
> }
>
> static Property ccw_device_properties[] = {
>diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
>index
>a06e91dfb318e3500324851488c56806fa46c08d..4b8ede701df9094972026
>2b6fc1b65f4e505e34d 100644
>--- a/hw/s390x/s390-ccw.c
>+++ b/hw/s390x/s390-ccw.c
>@@ -137,8 +137,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev,
>char *sysfsdev, Error **errp)
>         goto out_err;
>     }
>
>-    ck->realize(ccw_dev, &err);
>-    if (err) {
>+    if (!ck->realize(ccw_dev, &err)) {
>         goto out_err;
>     }
>
>--
>2.45.1