| Message ID | 20240522170107.289532-2-clg@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x/ccw: Error reporting cleanups | expand |
>-----Original Message----- >From: Cédric Le Goater <clg@redhat.com> >Subject: [PATCH 1/7] hw/s390x/ccw: Make s390_ccw_get_dev_info() return >a bool > >Since s390_ccw_get_dev_info() takes an 'Error **' argument, best >practices suggest to return a bool. See the qapi/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/s390-ccw.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > >diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c >index >5261e66724f1cc3157b9413b0d5fdf5289c92503..a06e91dfb318e35003248 >51488c56806fa46c08d 100644 >--- a/hw/s390x/s390-ccw.c >+++ b/hw/s390x/s390-ccw.c >@@ -71,7 +71,7 @@ IOInstEnding s390_ccw_store(SubchDev *sch) > return ret; > } > >-static void s390_ccw_get_dev_info(S390CCWDevice *cdev, >+static bool s390_ccw_get_dev_info(S390CCWDevice *cdev, > char *sysfsdev, > Error **errp) > { >@@ -84,12 +84,12 @@ static void s390_ccw_get_dev_info(S390CCWDevice >*cdev, > error_setg(errp, "No host device provided"); > error_append_hint(errp, > "Use -device vfio-ccw,sysfsdev=PATH_TO_DEVICE\n"); >- return; >+ return false; > } > > if (!realpath(sysfsdev, dev_path)) { > error_setg_errno(errp, errno, "Host device '%s' not found", sysfsdev); >- return; >+ return false; > } > > cdev->mdevid = g_path_get_basename(dev_path); >@@ -98,13 +98,14 @@ static void s390_ccw_get_dev_info(S390CCWDevice >*cdev, > tmp = g_path_get_basename(tmp_dir); > if (sscanf(tmp, "%2x.%1x.%4x", &cssid, &ssid, &devid) != 3) { > error_setg_errno(errp, errno, "Failed to read %s", tmp); >- return; >+ return false; > } > > cdev->hostid.cssid = cssid; > cdev->hostid.ssid = ssid; > cdev->hostid.devid = devid; > cdev->hostid.valid = true; >+ return true; > } > > static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error >**errp) >@@ -116,8 +117,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, >char *sysfsdev, Error **errp) > int ret; > Error *err = NULL; > >- s390_ccw_get_dev_info(cdev, sysfsdev, &err); >- if (err) { >+ if (!s390_ccw_get_dev_info(cdev, sysfsdev, &err)) { > goto out_err_propagate; > } > >-- >2.45.1
On 5/22/24 1:01 PM, Cédric Le Goater wrote: > Since s390_ccw_get_dev_info() takes an 'Error **' argument, best > practices suggest to return a bool. See the qapi/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/s390-ccw.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c > index 5261e66724f1cc3157b9413b0d5fdf5289c92503..a06e91dfb318e3500324851488c56806fa46c08d 100644 > --- a/hw/s390x/s390-ccw.c > +++ b/hw/s390x/s390-ccw.c > @@ -71,7 +71,7 @@ IOInstEnding s390_ccw_store(SubchDev *sch) > return ret; > } > > -static void s390_ccw_get_dev_info(S390CCWDevice *cdev, > +static bool s390_ccw_get_dev_info(S390CCWDevice *cdev, > char *sysfsdev, > Error **errp) > { > @@ -84,12 +84,12 @@ static void s390_ccw_get_dev_info(S390CCWDevice *cdev, > error_setg(errp, "No host device provided"); > error_append_hint(errp, > "Use -device vfio-ccw,sysfsdev=PATH_TO_DEVICE\n"); > - return; > + return false; > } > > if (!realpath(sysfsdev, dev_path)) { > error_setg_errno(errp, errno, "Host device '%s' not found", sysfsdev); > - return; > + return false; > } > > cdev->mdevid = g_path_get_basename(dev_path); > @@ -98,13 +98,14 @@ static void s390_ccw_get_dev_info(S390CCWDevice *cdev, > tmp = g_path_get_basename(tmp_dir); > if (sscanf(tmp, "%2x.%1x.%4x", &cssid, &ssid, &devid) != 3) { > error_setg_errno(errp, errno, "Failed to read %s", tmp); > - return; > + return false; > } > > cdev->hostid.cssid = cssid; > cdev->hostid.ssid = ssid; > cdev->hostid.devid = devid; > cdev->hostid.valid = true; > + return true; > } > > static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp) > @@ -116,8 +117,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp) > int ret; > Error *err = NULL; > > - s390_ccw_get_dev_info(cdev, sysfsdev, &err); > - if (err) { > + if (!s390_ccw_get_dev_info(cdev, sysfsdev, &err)) { > goto out_err_propagate; > } >
diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c index 5261e66724f1cc3157b9413b0d5fdf5289c92503..a06e91dfb318e3500324851488c56806fa46c08d 100644 --- a/hw/s390x/s390-ccw.c +++ b/hw/s390x/s390-ccw.c @@ -71,7 +71,7 @@ IOInstEnding s390_ccw_store(SubchDev *sch) return ret; } -static void s390_ccw_get_dev_info(S390CCWDevice *cdev, +static bool s390_ccw_get_dev_info(S390CCWDevice *cdev, char *sysfsdev, Error **errp) { @@ -84,12 +84,12 @@ static void s390_ccw_get_dev_info(S390CCWDevice *cdev, error_setg(errp, "No host device provided"); error_append_hint(errp, "Use -device vfio-ccw,sysfsdev=PATH_TO_DEVICE\n"); - return; + return false; } if (!realpath(sysfsdev, dev_path)) { error_setg_errno(errp, errno, "Host device '%s' not found", sysfsdev); - return; + return false; } cdev->mdevid = g_path_get_basename(dev_path); @@ -98,13 +98,14 @@ static void s390_ccw_get_dev_info(S390CCWDevice *cdev, tmp = g_path_get_basename(tmp_dir); if (sscanf(tmp, "%2x.%1x.%4x", &cssid, &ssid, &devid) != 3) { error_setg_errno(errp, errno, "Failed to read %s", tmp); - return; + return false; } cdev->hostid.cssid = cssid; cdev->hostid.ssid = ssid; cdev->hostid.devid = devid; cdev->hostid.valid = true; + return true; } static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp) @@ -116,8 +117,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp) int ret; Error *err = NULL; - s390_ccw_get_dev_info(cdev, sysfsdev, &err); - if (err) { + if (!s390_ccw_get_dev_info(cdev, sysfsdev, &err)) { goto out_err_propagate; }
Since s390_ccw_get_dev_info() takes an 'Error **' argument, best practices suggest to return a bool. See the qapi/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/s390-ccw.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)