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)
-- 
2.21.0

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?  :)