| Message ID | 20200317125741.15301-1-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Minor error handling cleanups | expand |
On 3/17/20 1:57 PM, Vladimir Sementsov-Ogievskiy wrote: > It's wrong to use same err object as errp parameter for several > function calls without intermediate checking for error: we'll crash if > try to set err object twice. Fix that. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > > Forgive me for sending this into your series, but seems it is very > appropriate. > > It's rephrasing of my > [PATCH v9 03/10] hw/sd/ssi-sd: fix error handling in ssi_sd_realize > for partI series but but without use of ERRP_AUTO_PROPAGATE. > > hw/sd/ssi-sd.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c > index 91db069212..829797b597 100644 > --- a/hw/sd/ssi-sd.c > +++ b/hw/sd/ssi-sd.c > @@ -255,13 +255,25 @@ static void ssi_sd_realize(SSISlave *d, Error **errp) > carddev = qdev_create(BUS(&s->sdbus), TYPE_SD_CARD); > if (dinfo) { > qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo), &err); > + if (err) { > + goto fail; > + } > } > + > object_property_set_bool(OBJECT(carddev), true, "spi", &err); > + if (err) { > + goto fail; > + } > + > object_property_set_bool(OBJECT(carddev), true, "realized", &err); > if (err) { > - error_setg(errp, "failed to init SD card: %s", error_get_pretty(err)); > - return; > + goto fail; > } > + > + return; > + > +fail: > + error_propagate_prepend(errp, err, "failed to init SD card: "); > } > > static void ssi_sd_reset(DeviceState *dev) > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > It's wrong to use same err object as errp parameter for several > function calls without intermediate checking for error: we'll crash if > try to set err object twice. Fix that. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > > Forgive me for sending this into your series, but seems it is very > appropriate. > > It's rephrasing of my > [PATCH v9 03/10] hw/sd/ssi-sd: fix error handling in ssi_sd_realize > for partI series but but without use of ERRP_AUTO_PROPAGATE. > > hw/sd/ssi-sd.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c > index 91db069212..829797b597 100644 > --- a/hw/sd/ssi-sd.c > +++ b/hw/sd/ssi-sd.c > @@ -255,13 +255,25 @@ static void ssi_sd_realize(SSISlave *d, Error **errp) > carddev = qdev_create(BUS(&s->sdbus), TYPE_SD_CARD); > if (dinfo) { > qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo), &err); > + if (err) { > + goto fail; > + } > } > + > object_property_set_bool(OBJECT(carddev), true, "spi", &err); > + if (err) { > + goto fail; > + } > + > object_property_set_bool(OBJECT(carddev), true, "realized", &err); > if (err) { > - error_setg(errp, "failed to init SD card: %s", error_get_pretty(err)); > - return; > + goto fail; > } > + > + return; > + > +fail: > + error_propagate_prepend(errp, err, "failed to init SD card: "); > } > > static void ssi_sd_reset(DeviceState *dev) Reviewed-by: Markus Armbruster <armbru@redhat.com>
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > It's wrong to use same err object as errp parameter for several > function calls without intermediate checking for error: we'll crash if > try to set err object twice. Fix that. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > > Forgive me for sending this into your series, but seems it is very > appropriate. > > It's rephrasing of my > [PATCH v9 03/10] hw/sd/ssi-sd: fix error handling in ssi_sd_realize > for partI series but but without use of ERRP_AUTO_PROPAGATE. Forgive? Thank you for extracting a bug fix for 5.0! Got more? :)
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c index 91db069212..829797b597 100644 --- a/hw/sd/ssi-sd.c +++ b/hw/sd/ssi-sd.c @@ -255,13 +255,25 @@ static void ssi_sd_realize(SSISlave *d, Error **errp) carddev = qdev_create(BUS(&s->sdbus), TYPE_SD_CARD); if (dinfo) { qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo), &err); + if (err) { + goto fail; + } } + object_property_set_bool(OBJECT(carddev), true, "spi", &err); + if (err) { + goto fail; + } + object_property_set_bool(OBJECT(carddev), true, "realized", &err); if (err) { - error_setg(errp, "failed to init SD card: %s", error_get_pretty(err)); - return; + goto fail; } + + return; + +fail: + error_propagate_prepend(errp, err, "failed to init SD card: "); } static void ssi_sd_reset(DeviceState *dev)
It's wrong to use same err object as errp parameter for several function calls without intermediate checking for error: we'll crash if try to set err object twice. Fix that. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- Forgive me for sending this into your series, but seems it is very appropriate. It's rephrasing of my [PATCH v9 03/10] hw/sd/ssi-sd: fix error handling in ssi_sd_realize for partI series but but without use of ERRP_AUTO_PROPAGATE. hw/sd/ssi-sd.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)