Re: [Qemu-devel] [PATCH v2 07/15] set_property(): push error handling to callers

[Luiz Capitulino]

On Tue,  5 Feb 2013 21:39:20 +0100
Laszlo Ersek <address@hidden> wrote:

> The return type can't be changed to void, because "set_property" must have
> type "qemu_opt_loopfunc".
> 
> Conversion status (call chains covered or substituted by error propagation
> marked with square brackets):

This is fine to be squashed with the previous patch because only
set_property() and its caller are touched. Hope I didn't say the
contrary for this patch in my last review :)

One more comment below.

> 
> do_device_add -> [qemu_find_opts -> error_report]
> do_device_add -> qdev_device_add -> qerror_report
> do_device_add -> qdev_device_add -> qbus_find -> qbus_find_recursive
>   -> qerror_report
> do_device_add -> qdev_device_add -> qbus_find -> qerror_report
> do_device_add -> qdev_device_add -> [set_property -> qdev_prop_parse
>   -> qerror_report_err]
> 
> Signed-off-by: Laszlo Ersek <address@hidden>
> ---
>  hw/qdev-monitor.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 545e66c..da32763 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -117,8 +117,7 @@ static int set_property(const char *name, const char 
> *value, void *opaque)
>  
>      qdev_prop_parse(ctx->dev, name, value, &err);
>      if (error_is_set(&err)) {
> -        qerror_report_err(err);
> -        error_free(err);
> +        error_propagate(ctx->errp, err);
>          return -1;
>      }
>      return 0;
> @@ -424,6 +423,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>      const char *driver, *path, *id;
>      DeviceState *qdev;
>      BusState *bus;
> +    Error *local_err = NULL;
>  
>      driver = qemu_opt_get(opts, "driver");
>      if (!driver) {
> @@ -488,9 +488,11 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>      }
>  
>      {
> -        PropertyContext ctx = { .dev = qdev, .errp = NULL };
> +        PropertyContext ctx = { .dev = qdev, .errp = &local_err };
>  
>          if (qemu_opt_foreach(opts, set_property, &ctx, 1) != 0) {

It's better to use local_err to check for error, as that's what
you use below.

> +            qerror_report_err(local_err);
> +            error_free(local_err);
>              qdev_free(qdev);
>              return NULL;
>          }