[Qemu-devel] [PATCH v2 04/15] loader-fit: Wean off error_printf()

[Markus Armbruster <armbru@redhat.com>]

load_fit() reports errors with error_printf() instead of
error_report().  Worse, it even reports errors it actually recovers
from, in fit_cfg_compatible() and fit_load_fdt().  Messed up in
initial commit 51b58561c1d.

Convert the helper functions for load_fit() to Error.  Make sure each
failure path sets an error.

Fix fit_cfg_compatible() and fit_load_fdt() not to report errors they
actually recover from.

Convert load_fit() to error_report().

Cc: Paul Burton <pburton@wavecomp.com>
Cc: Aleksandar Rikalo <arikalo@wavecomp.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/core/loader-fit.c | 62 +++++++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 26 deletions(-)

diff --git a/hw/core/loader-fit.c b/hw/core/loader-fit.c
index 447f60857d..f27b6af942 100644
--- a/hw/core/loader-fit.c
+++ b/hw/core/loader-fit.c
@@ -18,6 +18,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "qemu/units.h"
 #include "exec/memory.h"
 #include "hw/loader.h"
@@ -33,7 +34,7 @@
 #define FIT_LOADER_MAX_PATH (128)
 
 static const void *fit_load_image_alloc(const void *itb, const char *name,
-                                        int *poff, size_t *psz)
+                                        int *poff, size_t *psz, Error **errp)
 {
     const void *data;
     const char *comp;
@@ -46,6 +47,7 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
 
     off = fdt_path_offset(itb, path);
     if (off < 0) {
+        error_setg(errp, "can't find node %s", path);
         return NULL;
     }
     if (poff) {
@@ -54,6 +56,7 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
 
     data = fdt_getprop(itb, off, "data", &sz);
     if (!data) {
+        error_setg(errp, "can't get %s/data", path);
         return NULL;
     }
 
@@ -73,7 +76,7 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
 
         uncomp_len = gunzip(uncomp_data, uncomp_len, (void *) data, sz);
         if (uncomp_len < 0) {
-            error_printf("unable to decompress %s image\n", name);
+            error_setg(errp, "unable to decompress %s image", name);
             g_free(uncomp_data);
             return NULL;
         }
@@ -85,18 +88,19 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
         return data;
     }
 
-    error_printf("unknown compression '%s'\n", comp);
+    error_setg(errp, "unknown compression '%s'", comp);
     return NULL;
 }
 
 static int fit_image_addr(const void *itb, int img, const char *name,
-                          hwaddr *addr)
+                          hwaddr *addr, Error **errp)
 {
     const void *prop;
     int len;
 
     prop = fdt_getprop(itb, img, name, &len);
     if (!prop) {
+        error_setg(errp, "can't find %s address", name);
         return -ENOENT;
     }
 
@@ -108,13 +112,14 @@ static int fit_image_addr(const void *itb, int img, const char *name,
         *addr = fdt64_to_cpu(*(fdt64_t *)prop);
         return 0;
     default:
-        error_printf("invalid %s address length %d\n", name, len);
+        error_setg(errp, "invalid %s address length %d", name, len);
         return -EINVAL;
     }
 }
 
 static int fit_load_kernel(const struct fit_loader *ldr, const void *itb,
-                           int cfg, void *opaque, hwaddr *pend)
+                           int cfg, void *opaque, hwaddr *pend,
+                           Error **errp)
 {
     const char *name;
     const void *data;
@@ -126,26 +131,26 @@ static int fit_load_kernel(const struct fit_loader *ldr, const void *itb,
 
     name = fdt_getprop(itb, cfg, "kernel", NULL);
     if (!name) {
-        error_printf("no kernel specified by FIT configuration\n");
+        error_setg(errp, "no kernel specified by FIT configuration");
         return -EINVAL;
     }
 
-    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz);
+    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz, errp);
     if (!data) {
-        error_printf("unable to load kernel image from FIT\n");
+        error_prepend(errp, "unable to load kernel image from FIT: ");
         return -EINVAL;
     }
 
-    err = fit_image_addr(itb, img_off, "load", &load_addr);
+    err = fit_image_addr(itb, img_off, "load", &load_addr, errp);
     if (err) {
-        error_printf("unable to read kernel load address from FIT\n");
+        error_prepend(errp, "unable to read kernel load address from FIT: ");
         ret = err;
         goto out;
     }
 
-    err = fit_image_addr(itb, img_off, "entry", &entry_addr);
+    err = fit_image_addr(itb, img_off, "entry", &entry_addr, errp);
     if (err) {
-        error_printf("unable to read kernel entry address from FIT\n");
+        error_prepend(errp, "unable to read kernel entry address from FIT: ");
         ret = err;
         goto out;
     }
@@ -172,7 +177,7 @@ out:
 
 static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
                         int cfg, void *opaque, const void *match_data,
-                        hwaddr kernel_end)
+                        hwaddr kernel_end, Error **errp)
 {
     const char *name;
     const void *data;
@@ -187,16 +192,18 @@ static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
         return 0;
     }
 
-    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz);
+    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz, errp);
     if (!data) {
-        error_printf("unable to load FDT image from FIT\n");
+        error_prepend(errp, "unable to load FDT image from FIT: ");
         return -EINVAL;
     }
 
-    err = fit_image_addr(itb, img_off, "load", &load_addr);
+    err = fit_image_addr(itb, img_off, "load", &load_addr, errp);
     if (err == -ENOENT) {
         load_addr = ROUND_UP(kernel_end, 64 * KiB) + (10 * MiB);
+        error_free(*errp);
     } else if (err) {
+        error_prepend(errp, "unable to read FDT load address from FIT: ");
         ret = err;
         goto out;
     }
@@ -229,7 +236,7 @@ static bool fit_cfg_compatible(const void *itb, int cfg, const char *compat)
         return false;
     }
 
-    fdt = fit_load_image_alloc(itb, fdt_name, NULL, NULL);
+    fdt = fit_load_image_alloc(itb, fdt_name, NULL, NULL, NULL);
     if (!fdt) {
         return false;
     }
@@ -252,11 +259,12 @@ out:
 
 int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque)
 {
+    Error *err = NULL;
     const struct fit_loader_match *match;
     const void *itb, *match_data = NULL;
     const char *def_cfg_name;
     char path[FIT_LOADER_MAX_PATH];
-    int itb_size, configs, cfg_off, off, err;
+    int itb_size, configs, cfg_off, off;
     hwaddr kernel_end;
     int ret;
 
@@ -267,6 +275,7 @@ int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque)
 
     configs = fdt_path_offset(itb, "/configurations");
     if (configs < 0) {
+        error_report("can't find node /configurations");
         ret = configs;
         goto out;
     }
@@ -301,20 +310,21 @@ int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque)
     }
 
     if (cfg_off < 0) {
-        /* couldn't find a configuration to use */
+        error_report("can't find configuration");
         ret = cfg_off;
         goto out;
     }
 
-    err = fit_load_kernel(ldr, itb, cfg_off, opaque, &kernel_end);
-    if (err) {
-        ret = err;
+    ret = fit_load_kernel(ldr, itb, cfg_off, opaque, &kernel_end, &err);
+    if (ret) {
+        error_report_err(err);
         goto out;
     }
 
-    err = fit_load_fdt(ldr, itb, cfg_off, opaque, match_data, kernel_end);
-    if (err) {
-        ret = err;
+    ret = fit_load_fdt(ldr, itb, cfg_off, opaque, match_data, kernel_end,
+                       &err);
+    if (ret) {
+        error_report_err(err);
         goto out;
     }
 
-- 
2.17.2