All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: hreitz@redhat.com, qemu-block@nongnu.org, Kevin Wolf <kwolf@redhat.com>
Subject: [PATCH v3 17/19] cutils: Use parse_uint in qemu_strtosz for negative rejection
Date: Mon, 22 May 2023 14:04:39 -0500	[thread overview]
Message-ID: <20230522190441.64278-18-eblake@redhat.com> (raw)
In-Reply-To: <20230522190441.64278-1-eblake@redhat.com>

Rather than open-coding two different ways to check for an unwanted
negative sign, reuse the same code in both functions.  That way, if we
decide down the road to accept "-0" instead of rejecting it, we have
fewer places to change.  Also, it means we now get ERANGE instead of
EINVAL for negative values in qemu_strtosz, which is reasonable for
what it represents.  This in turn changes the expected output of a
couple of iotests.

The change is not quite complete: negative fractional scaled values
can trip us up.  This will be fixed in a later patch addressing other
issues with fractional scaled values.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
---

v3: also fix iotests, commit message tweaked, R-b added
---
 tests/unit/test-cutils.c         | 7 +++----
 util/cutils.c                    | 8 ++------
 tests/qemu-iotests/049.out       | 7 ++-----
 tests/qemu-iotests/178.out.qcow2 | 3 +--
 tests/qemu-iotests/178.out.raw   | 3 +--
 5 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index adc9878f93d..8bdae856827 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -3519,10 +3519,9 @@ static void test_qemu_strtosz_trailing(void)
 static void test_qemu_strtosz_erange(void)
 {
     /* FIXME negative values fit better as ERANGE */
-    do_strtosz(" -0", -EINVAL /* FIXME -ERANGE */, 0, 0 /* FIXME 3 */);
-    do_strtosz("-1", -EINVAL /* FIXME -ERANGE */, 0, 0 /* FIXME 2 */);
-    do_strtosz_full("-2M", qemu_strtosz, -EINVAL /* FIXME -ERANGE */, 0,
-                    0 /* FIXME 2 */, -EINVAL, 0);
+    do_strtosz(" -0", -ERANGE, 0, 3);
+    do_strtosz("-1", -ERANGE, 0, 2);
+    do_strtosz_full("-2M", qemu_strtosz, -ERANGE, 0, 2, -EINVAL, 0);
     do_strtosz(" -.0", -EINVAL /* FIXME -ERANGE */, 0, 0 /* FIXME 4 */);
     do_strtosz_full("-.1k", qemu_strtosz, -EINVAL /* FIXME -ERANGE */, 0,
                     0 /* FIXME 3 */, -EINVAL, 0);
diff --git a/util/cutils.c b/util/cutils.c
index edfb71a2171..e3a49209a94 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -201,6 +201,7 @@ static int64_t suffix_mul(char suffix, int64_t unit)
  * - hex with scaling suffix, such as 0x20M
  * - octal, such as 08
  * - fractional hex, such as 0x1.8
+ * - negative values, including -0
  * - floating point exponents, such as 1e3
  *
  * The end pointer will be returned in *end, if not NULL.  If there is
@@ -226,15 +227,10 @@ static int do_strtosz(const char *nptr, const char **end,
     int64_t mul;

     /* Parse integral portion as decimal. */
-    retval = qemu_strtou64(nptr, &endptr, 10, &val);
+    retval = parse_uint(nptr, &endptr, 10, &val);
     if (retval) {
         goto out;
     }
-    if (memchr(nptr, '-', endptr - nptr) != NULL) {
-        endptr = nptr;
-        retval = -EINVAL;
-        goto out;
-    }
     if (val == 0 && (*endptr == 'x' || *endptr == 'X')) {
         /* Input looks like hex; reparse, and insist on no fraction or suffix. */
         retval = qemu_strtou64(nptr, &endptr, 16, &val);
diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
index 8719c91b483..34e1b452e6e 100644
--- a/tests/qemu-iotests/049.out
+++ b/tests/qemu-iotests/049.out
@@ -92,13 +92,10 @@ Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off comp
 == 3. Invalid sizes ==

 qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1024
-qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for
-qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
+qemu-img: Invalid image size specified. Must be between 0 and 9223372036854775807.

 qemu-img create -f qcow2 -o size=-1024 TEST_DIR/t.qcow2
-qemu-img: TEST_DIR/t.qcow2: Parameter 'size' expects a non-negative number below 2^64
-Optional suffix k, M, G, T, P or E means kilo-, mega-, giga-, tera-, peta-
-and exabytes, respectively.
+qemu-img: TEST_DIR/t.qcow2: Value '-1024' is out of range for parameter 'size'

 qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1k
 qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for
diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2
index 0d51fe401ec..fe193fd5f4f 100644
--- a/tests/qemu-iotests/178.out.qcow2
+++ b/tests/qemu-iotests/178.out.qcow2
@@ -13,8 +13,7 @@ qemu-img: Invalid option list: ,
 qemu-img: Invalid parameter 'snapshot.foo'
 qemu-img: Failed in parsing snapshot param 'snapshot.foo=bar'
 qemu-img: --output must be used with human or json as argument.
-qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for
-qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
+qemu-img: Invalid image size specified. Must be between 0 and 9223372036854775807.
 qemu-img: Unknown file format 'foo'

 == Size calculation for a new file (human) ==
diff --git a/tests/qemu-iotests/178.out.raw b/tests/qemu-iotests/178.out.raw
index 116241ddef2..445e460fad9 100644
--- a/tests/qemu-iotests/178.out.raw
+++ b/tests/qemu-iotests/178.out.raw
@@ -13,8 +13,7 @@ qemu-img: Invalid option list: ,
 qemu-img: Invalid parameter 'snapshot.foo'
 qemu-img: Failed in parsing snapshot param 'snapshot.foo=bar'
 qemu-img: --output must be used with human or json as argument.
-qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for
-qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
+qemu-img: Invalid image size specified. Must be between 0 and 9223372036854775807.
 qemu-img: Unknown file format 'foo'

 == Size calculation for a new file (human) ==
-- 
2.40.1



  parent reply	other threads:[~2023-05-22 19:11 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-22 19:04 [PATCH v3 00/19] Fix qemu_strtosz() read-out-of-bounds Eric Blake
2023-05-22 19:04 ` [PATCH v3 01/19] test-cutils: Avoid g_assert in unit tests Eric Blake
2023-05-22 19:04 ` [PATCH v3 02/19] test-cutils: Use g_assert_cmpuint where appropriate Eric Blake
2023-05-22 19:04 ` [PATCH v3 03/19] test-cutils: Test integral qemu_strto* value on failures Eric Blake
2023-05-22 19:04 ` [PATCH v3 04/19] test-cutils: Test more integer corner cases Eric Blake
2023-05-23 17:25   ` Hanna Czenczek
2023-05-23 19:19     ` Eric Blake
2023-05-22 19:04 ` [PATCH v3 05/19] cutils: Fix wraparound parsing in qemu_strtoui Eric Blake
2023-05-23 17:36   ` Hanna Czenczek
2023-05-22 19:04 ` [PATCH v3 06/19] cutils: Document differences between parse_uint and qemu_strtou64 Eric Blake
2023-05-22 19:04 ` [PATCH v3 07/19] cutils: Adjust signature of parse_uint[_full] Eric Blake
2023-05-22 19:04 ` [PATCH v3 08/19] cutils: Allow NULL endptr in parse_uint() Eric Blake
2023-05-22 19:04 ` [PATCH v3 09/19] test-cutils: Add coverage of qemu_strtod Eric Blake
2023-05-22 19:04 ` [PATCH v3 10/19] test-cutils: Prepare for upcoming semantic change in qemu_strtosz Eric Blake
2023-05-22 19:04 ` [PATCH v3 11/19] test-cutils: Refactor qemu_strtosz tests for less boilerplate Eric Blake
2023-05-22 19:04 ` [PATCH v3 12/19] cutils: Allow NULL str in qemu_strtosz Eric Blake
2023-05-22 19:04 ` [PATCH v3 13/19] numa: Check for qemu_strtosz_MiB error Eric Blake
2023-05-22 19:04 ` [PATCH v3 14/19] test-cutils: Add more coverage to qemu_strtosz Eric Blake
2023-05-22 19:04 ` [PATCH v3 15/19] cutils: Set value in all qemu_strtosz* error paths Eric Blake
2023-05-22 19:04 ` [PATCH v3 16/19] cutils: Set value in all integral qemu_strto* " Eric Blake
2023-05-22 19:04 ` Eric Blake [this message]
2023-05-22 19:04 ` [PATCH v3 18/19] cutils: Improve qemu_strtod* " Eric Blake
2023-05-22 19:04 ` [PATCH v3 19/19] cutils: Improve qemu_strtosz handling of fractions Eric Blake
2023-06-01 21:46   ` Eric Blake
2023-06-01 21:28 ` [PATCH v3 00/19] Fix qemu_strtosz() read-out-of-bounds Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230522190441.64278-18-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.