From bd6fee9f1263dc5ba487c7ac57d33a727af63c00 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Wed, 15 Jun 2016 19:27:15 +0200
Subject: log: Fix qemu_set_dfilter_ranges() error reporting

g_error() is not an acceptable way to report errors to the user:

    $ qemu-system-x86_64 -dfilter 1000+0

    ** (process:17187): ERROR **: Failed to parse range in: 1000+0
    Trace/breakpoint trap (core dumped)

g_assert() isn't, either:

    $ qemu-system-x86_64 -dfilter 1000x+64
    **
    ERROR:/work/armbru/qemu/util/log.c:180:qemu_set_dfilter_ranges: assertion failed: (e == range_op)
    Aborted (core dumped)

Convert qemu_set_dfilter_ranges() to Error.  Rework its deeply nested
control flow.  Touch up the error messages.  Call it with
&error_fatal.

This also permits testing without a subprocess, so do that.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1466011636-6112-3-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 util/log.c | 113 +++++++++++++++++++++++++++++++------------------------------
 1 file changed, 57 insertions(+), 56 deletions(-)

(limited to 'util')

diff --git a/util/log.c b/util/log.c
index 6f45e0a26c..fcf85c6253 100644
--- a/util/log.c
+++ b/util/log.c
@@ -22,6 +22,7 @@
 #include "qemu/log.h"
 #include "qemu/range.h"
 #include "qemu/error-report.h"
+#include "qapi/error.h"
 #include "qemu/cutils.h"
 #include "trace/control.h"
 
@@ -142,75 +143,75 @@ bool qemu_log_in_addr_range(uint64_t addr)
 }
 
 
-void qemu_set_dfilter_ranges(const char *filter_spec)
+void qemu_set_dfilter_ranges(const char *filter_spec, Error **errp)
 {
     gchar **ranges = g_strsplit(filter_spec, ",", 0);
+    int i;
 
     if (debug_regions) {
         g_array_unref(debug_regions);
         debug_regions = NULL;
     }
 
-    if (ranges) {
-        gchar **next = ranges;
-        gchar *r = *next++;
+    debug_regions = g_array_sized_new(FALSE, FALSE,
+                                      sizeof(Range), g_strv_length(ranges));
+    for (i = 0; ranges[i]; i++) {
+        const char *r = ranges[i];
+        const char *range_op, *r2, *e;
+        uint64_t r1val, r2val;
+        struct Range range;
 
-        debug_regions = g_array_sized_new(FALSE, FALSE,
-                                          sizeof(Range), g_strv_length(ranges));
-        while (r) {
-            char *range_op = strstr(r, "-");
-            char *r2 = range_op ? range_op + 1 : NULL;
-            if (!range_op) {
-                range_op = strstr(r, "+");
-                r2 = range_op ? range_op + 1 : NULL;
-            }
-            if (!range_op) {
-                range_op = strstr(r, "..");
-                r2 = range_op ? range_op + 2 : NULL;
-            }
-            if (range_op) {
-                const char *e = NULL;
-                uint64_t r1val, r2val;
-
-                if ((qemu_strtoull(r, &e, 0, &r1val) == 0) &&
-                    (qemu_strtoull(r2, NULL, 0, &r2val) == 0) &&
-                    r2val > 0) {
-                    struct Range range;
-
-                    g_assert(e == range_op);
+        range_op = strstr(r, "-");
+        r2 = range_op ? range_op + 1 : NULL;
+        if (!range_op) {
+            range_op = strstr(r, "+");
+            r2 = range_op ? range_op + 1 : NULL;
+        }
+        if (!range_op) {
+            range_op = strstr(r, "..");
+            r2 = range_op ? range_op + 2 : NULL;
+        }
+        if (!range_op) {
+            error_setg(errp, "Bad range specifier");
+            goto out;
+        }
 
-                    switch (*range_op) {
-                    case '+':
-                    {
-                        range.begin = r1val;
-                        range.end = r1val + (r2val - 1);
-                        break;
-                    }
-                    case '-':
-                    {
-                        range.end = r1val;
-                        range.begin = r1val - (r2val - 1);
-                        break;
-                    }
-                    case '.':
-                        range.begin = r1val;
-                        range.end = r2val;
-                        break;
-                    default:
-                        g_assert_not_reached();
-                    }
-                    g_array_append_val(debug_regions, range);
+        if (qemu_strtoull(r, &e, 0, &r1val)
+            || e != range_op) {
+            error_setg(errp, "Invalid number to the left of %.*s",
+                       (int)(r2 - range_op), range_op);
+            goto out;
+        }
+        if (qemu_strtoull(r2, NULL, 0, &r2val)) {
+            error_setg(errp, "Invalid number to the right of %.*s",
+                       (int)(r2 - range_op), range_op);
+            goto out;
+        }
+        if (r2val == 0) {
+            error_setg(errp, "Invalid range");
+            goto out;
+        }
 
-                } else {
-                    g_error("Failed to parse range in: %s", r);
-                }
-            } else {
-                g_error("Bad range specifier in: %s", r);
-            }
-            r = *next++;
+        switch (*range_op) {
+        case '+':
+            range.begin = r1val;
+            range.end = r1val + (r2val - 1);
+            break;
+        case '-':
+            range.end = r1val;
+            range.begin = r1val - (r2val - 1);
+            break;
+        case '.':
+            range.begin = r1val;
+            range.end = r2val;
+            break;
+        default:
+            g_assert_not_reached();
         }
-        g_strfreev(ranges);
+        g_array_append_val(debug_regions, range);
     }
+out:
+    g_strfreev(ranges);
 }
 
 /* fflush() the log file */
-- 
cgit v1.2.3