| Message ID | 20240522170107.289532-3-clg@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x/ccw: Error reporting cleanups | expand |
>-----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
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; > } >
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; }
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(-)