diff mbox

[v2,12/15] qbus_find(): propagate error handling / consumption to callers

Message ID 1360096768-8863-13-git-send-email-lersek@redhat.com
State New
Headers show

Commit Message

Laszlo Ersek Feb. 5, 2013, 8:39 p.m. UTC 
Conversion status (call chains covered or substituted by error propagation
marked with square brackets):

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 <lersek@redhat.com>
---
 hw/qdev-monitor.c |   30 ++++++++++++++++--------------
 1 files changed, 16 insertions(+), 14 deletions(-)

Comments

Markus Armbruster March 20, 2013, 3:32 p.m. UTC | #1 
Laszlo Ersek <lersek@redhat.com> writes:

> Conversion status (call chains covered or substituted by error propagation
> marked with square brackets):
>
> 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 <lersek@redhat.com>
> ---
>  hw/qdev-monitor.c |   30 ++++++++++++++++--------------
>  1 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 728aa24..80fc9f8 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -371,20 +371,20 @@ static BusState *qbus_find(const char *path, Error **errp)
>          bus = sysbus_get_default();
>          pos = 0;
>      } else {
> -        Error *err = NULL;
> +        Error *named_bus_full = NULL;
>  
>          if (sscanf(path, "%127[^/]%n", elem, &len) != 1) {
>              assert(!path[0]);
>              elem[0] = len = 0;
>          }
> -        bus = qbus_find_recursive(sysbus_get_default(), elem, NULL, &err);
> +        bus = qbus_find_recursive(sysbus_get_default(), elem, NULL,
> +                                  &named_bus_full);
>          if (!bus) {
> -            if (error_is_set(&err)) {
> -                qerror_report_err(err);
> -                error_free(err);
> -            } else {
> -                qerror_report(QERR_BUS_NOT_FOUND, elem);
> -            }
> +            Error *not_found = NULL;
> +
> +            error_set(&not_found, QERR_BUS_NOT_FOUND, elem);
> +            error_propagate(errp, named_bus_full);
> +            error_propagate(errp, not_found);
>              return NULL;
>          }
>          pos = len;

Cute trick!

Would look even cuter if we had a function returning a newly minted
error object.

[...]
diff mbox

Patch

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 728aa24..80fc9f8 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -371,20 +371,20 @@  static BusState *qbus_find(const char *path, Error **errp)
         bus = sysbus_get_default();
         pos = 0;
     } else {
-        Error *err = NULL;
+        Error *named_bus_full = NULL;
 
         if (sscanf(path, "%127[^/]%n", elem, &len) != 1) {
             assert(!path[0]);
             elem[0] = len = 0;
         }
-        bus = qbus_find_recursive(sysbus_get_default(), elem, NULL, &err);
+        bus = qbus_find_recursive(sysbus_get_default(), elem, NULL,
+                                  &named_bus_full);
         if (!bus) {
-            if (error_is_set(&err)) {
-                qerror_report_err(err);
-                error_free(err);
-            } else {
-                qerror_report(QERR_BUS_NOT_FOUND, elem);
-            }
+            Error *not_found = NULL;
+
+            error_set(&not_found, QERR_BUS_NOT_FOUND, elem);
+            error_propagate(errp, named_bus_full);
+            error_propagate(errp, not_found);
             return NULL;
         }
         pos = len;
@@ -407,7 +407,7 @@  static BusState *qbus_find(const char *path, Error **errp)
         pos += len;
         dev = qbus_find_dev(bus, elem);
         if (!dev) {
-            qerror_report(QERR_DEVICE_NOT_FOUND, elem);
+            error_set(errp, QERR_DEVICE_NOT_FOUND, elem);
             if (!monitor_cur_is_qmp()) {
                 qbus_list_dev(bus);
             }
@@ -423,12 +423,12 @@  static BusState *qbus_find(const char *path, Error **errp)
              * one child bus accept it nevertheless */
             switch (dev->num_child_bus) {
             case 0:
-                qerror_report(QERR_DEVICE_NO_BUS, elem);
+                error_set(errp, QERR_DEVICE_NO_BUS, elem);
                 return NULL;
             case 1:
                 return QLIST_FIRST(&dev->child_bus);
             default:
-                qerror_report(QERR_DEVICE_MULTIPLE_BUSSES, elem);
+                error_set(errp, QERR_DEVICE_MULTIPLE_BUSSES, elem);
                 if (!monitor_cur_is_qmp()) {
                     qbus_list_bus(dev);
                 }
@@ -444,7 +444,7 @@  static BusState *qbus_find(const char *path, Error **errp)
         pos += len;
         bus = qbus_find_bus(dev, elem);
         if (!bus) {
-            qerror_report(QERR_BUS_NOT_FOUND, elem);
+            error_set(errp, QERR_BUS_NOT_FOUND, elem);
             if (!monitor_cur_is_qmp()) {
                 qbus_list_bus(dev);
             }
@@ -489,8 +489,10 @@  DeviceState *qdev_device_add(QemuOpts *opts)
     /* find bus */
     path = qemu_opt_get(opts, "bus");
     if (path != NULL) {
-        bus = qbus_find(path, NULL);
-        if (!bus) {
+        bus = qbus_find(path, &local_err);
+        if (error_is_set(&local_err)) {
+            qerror_report_err(local_err);
+            error_free(local_err);
             return NULL;
         }
         if (!object_dynamic_cast(OBJECT(bus), k->bus_type)) {