qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 1/1] qemu_img: add cvtnum_full to print error reports


From: Eric Blake
Subject: Re: [PATCH v3 1/1] qemu_img: add cvtnum_full to print error reports
Date: Wed, 13 May 2020 10:57:46 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0

On 5/13/20 8:36 AM, Eyal Moscovici wrote:
All calls to cvtnum check the return value and print the same error message more
or less. And so error reporting moved to cvtnum_full to reduce code
duplication and provide a single error message. Additionally, cvtnum now wraps
cvtnum_full with the existing default range of 0 to MAX_INT64.

Acked-by: Mark Kanda <address@hidden>
Signed-off-by: Eyal Moscovici <address@hidden>
---

-static int64_t cvtnum(const char *s)
+static int64_t cvtnum_full(const char *name, const char *value, int64_t min,
+                           int64_t max)
  {
      int err;
-    uint64_t value;
-
-    err = qemu_strtosz(s, NULL, &value);
-    if (err < 0) {
+    uint64_t res;
+
+    err = qemu_strtosz(value, NULL, &res);
+    if (err < 0 && err != -ERANGE) {
+        error_report("Invalid %s specified. You may use "
+                     "k, M, G, T, P or E suffixes for ", name);
+        error_report("kilobytes, megabytes, gigabytes, terabytes, "
+                     "petabytes and exabytes.");

The new common message is in terms of bytes, even though not all callers are specifically related to bytes. I don't think it's a show-stopper, though, merely a quality of error message, and I don't have any ideas for how to improve it.

@@ -4506,10 +4508,9 @@ static int img_dd_count(const char *arg,
                          struct DdIo *in, struct DdIo *out,
                          struct DdInfo *dd)
  {
-    dd->count = cvtnum(arg);
+    dd->count = cvtnum("count", arg);
if (dd->count < 0) {
-        error_report("invalid number: '%s'", arg);

For example, the count argument to dd is not really about bytes, but repetitions. So the error message is now more informative (what suffixes you can use) but also less accurate ("invalid number" was vague but at least correct).

I think the common code is worth the corner case regressions, so:

Reviewed-by: Eric Blake <address@hidden>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




reply via email to

[Prev in Thread] Current Thread [Next in Thread]