From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: hreitz@redhat.com, armbru@redhat.com, richard.henderson@linaro.org
Subject: [PATCH v2 10/19] test-cutils: Prepare for upcoming semantic change in qemu_strtosz
Date: Thu, 11 May 2023 21:10:24 -0500 [thread overview]
Message-ID: <20230512021033.1378730-11-eblake@redhat.com> (raw)
In-Reply-To: <20230512021033.1378730-1-eblake@redhat.com>
A quick search for 'qemu_strtosz' in the code base shows that outside
of the testsuite, the ONLY place that passes a non-NULL pointer to
@endptr of any variant of a size parser is in hmp.c (the 'o' parser of
monitor_parse_arguments), and that particular caller warns of
"extraneous characters at the end of line" unless the trailing bytes
are purely whitespace. Thus, it makes no semantic difference at the
high level whether we parse "1.5e1k" as "1" + ".5e1" + "k" (an attempt
to use scientific notation in strtod with a scaling suffix of 'k' with
no trailing junk, but which qemu_strtosz says should fail with
EINVAL), or as "1.5e" + "1k" (a valid size with scaling suffix of 'e'
for exabytes, followed by two junk bytes) - either way, any user
passing such a string will get an error message about a parse failure.
However, an upcoming patch to qemu_strtosz will fix other corner case
bugs in handling the fractional portion of a size, and in doing so, it
is easier to declare that qemu_strtosz() itself stops parsing at the
first 'e' rather than blindly consuming whatever strtod() will
recognize. Once that is fixed, the difference will be visible at the
low level (getting a valid parse with trailing garbage when @endptr is
non-NULL, while continuing to get -EINVAL when @endptr is NULL); this
is easier to demonstrate by moving the affected strings from
test_qemu_strtosz_invalid() (which declares them as always -EINVAL) to
test_qemu_strtosz_trailing() (where @endptr affects behavior, for now
with FIXME comments).
Note that a similar argument could be made for having "0x1.5" or
"0x1M" parse as 0x1 with ".5" or "M" as trailing junk, instead of
blindly treating it as -EINVAL; however, as these cases do not suffer
from the same problems as floating point, they are not worth changing
at this time.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
---
tests/unit/test-cutils.c | 42 ++++++++++++++++++++++++++--------------
1 file changed, 27 insertions(+), 15 deletions(-)
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 1763839a157..fe9be690faf 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -3440,21 +3440,6 @@ static void test_qemu_strtosz_invalid(void)
g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
- /* No floating point exponents */
- str = "1.5e1k";
- endptr = NULL;
- err = qemu_strtosz(str, &endptr, &res);
- g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
- g_assert_true(endptr == str);
-
- str = "1.5E+0k";
- endptr = NULL;
- err = qemu_strtosz(str, &endptr, &res);
- g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
- g_assert_true(endptr == str);
-
/* No hex fractions */
str = "0x1.8k";
endptr = NULL;
@@ -3558,6 +3543,33 @@ static void test_qemu_strtosz_trailing(void)
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
+
+ /* FIXME should stop parse after 'e'. No floating point exponents */
+ str = "1.5e1k";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL /* FIXME 0 */);
+ g_assert_cmphex(res, ==, 0xbaadf00d /* FIXME EiB * 1.5 */);
+ g_assert_true(endptr == str /* FIXME + 4 */);
+
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0xbaadf00d);
+
+ str = "1.5E+0k";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL /* FIXME 0 */);
+ g_assert_cmphex(res, ==, 0xbaadf00d /* FIXME EiB * 1.5 */);
+ g_assert_true(endptr == str /* FIXME + 4 */);
+
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
}
static void test_qemu_strtosz_erange(void)
--
2.40.1
next prev parent reply other threads:[~2023-05-12 2:12 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-12 2:10 [PATCH v2 00/19] Fix qemu_strtosz() read-out-of-bounds Eric Blake
2023-05-12 2:10 ` [PATCH v2 01/19] test-cutils: Avoid g_assert in unit tests Eric Blake
2023-05-12 3:20 ` Philippe Mathieu-Daudé
2023-05-12 12:11 ` Eric Blake
2023-05-12 2:10 ` [PATCH v2 02/19] test-cutils: Use g_assert_cmpuint where appropriate Eric Blake
2023-05-12 2:10 ` [PATCH v2 03/19] test-cutils: Test integral qemu_strto* value on failures Eric Blake
2023-05-12 2:10 ` [PATCH v2 04/19] test-cutils: Test more integer corner cases Eric Blake
2023-05-19 14:27 ` Hanna Czenczek
2023-05-19 15:17 ` Eric Blake
2023-05-12 2:10 ` [PATCH v2 05/19] cutils: Fix wraparound parsing in qemu_strtoui Eric Blake
2023-05-18 13:34 ` Eric Blake
2023-05-19 14:42 ` Hanna Czenczek
2023-05-19 16:31 ` Eric Blake
2023-05-12 2:10 ` [PATCH v2 06/19] cutils: Document differences between parse_uint and qemu_strtou64 Eric Blake
2023-05-19 14:44 ` Hanna Czenczek
2023-05-12 2:10 ` [PATCH v2 07/19] cutils: Adjust signature of parse_uint[_full] Eric Blake
2023-05-12 16:25 ` Eric Blake
2023-05-19 14:51 ` Hanna Czenczek
2023-05-12 2:10 ` [PATCH v2 08/19] cutils: Allow NULL endptr in parse_uint() Eric Blake
2023-05-12 16:44 ` Eric Blake
2023-05-19 14:54 ` Hanna Czenczek
2023-05-12 2:10 ` [PATCH v2 09/19] test-cutils: Add coverage of qemu_strtod Eric Blake
2023-05-19 15:05 ` Hanna Czenczek
2023-05-19 17:52 ` Eric Blake
2023-05-22 10:56 ` Hanna Czenczek
2023-05-22 12:59 ` Eric Blake
2023-05-12 2:10 ` Eric Blake [this message]
2023-05-12 2:10 ` [PATCH v2 11/19] test-cutils: Refactor qemu_strtosz tests for less boilerplate Eric Blake
2023-05-19 15:13 ` Hanna Czenczek
2023-05-19 17:54 ` Eric Blake
2023-05-12 2:10 ` [PATCH v2 12/19] cutils: Allow NULL str in qemu_strtosz Eric Blake
2023-05-12 3:25 ` Philippe Mathieu-Daudé
2023-05-19 15:15 ` Hanna Czenczek
2023-05-12 2:10 ` [PATCH v2 13/19] numa: Check for qemu_strtosz_MiB error Eric Blake
2023-05-12 2:10 ` [PATCH v2 14/19] test-cutils: Add more coverage to qemu_strtosz11; rgb:1e1e/1e1e/1e1e Eric Blake
2023-05-19 15:26 ` [PATCH v2 14/19] test-cutils: Add more coverage to qemu_strtosz11;rgb:1e1e/1e1e/1e1e Hanna Czenczek
2023-05-19 18:02 ` Eric Blake
2023-05-12 2:10 ` [PATCH v2 15/19] cutils: Set value in all qemu_strtosz* error paths Eric Blake
2023-05-19 15:29 ` Hanna Czenczek
2023-05-12 2:10 ` [PATCH v2 16/19] cutils: Set value in all integral qemu_strto* " Eric Blake
2023-05-12 2:10 ` [PATCH v2 17/19] cutils: Use parse_uint in qemu_strtosz for negative rejection Eric Blake
2023-05-12 19:34 ` Eric Blake
2023-05-19 15:32 ` Hanna Czenczek
2023-05-12 2:10 ` [PATCH v2 18/19] cutils: Improve qemu_strtod* error paths Eric Blake
2023-05-18 13:47 ` Eric Blake
2023-05-12 2:10 ` [PATCH v2 19/19] cutils: Improve qemu_strtosz handling of fractions Eric Blake
2023-05-19 15:36 ` Hanna Czenczek
2023-05-12 12:24 ` [PATCH v2 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=20230512021033.1378730-11-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=armbru@redhat.com \
--cc=hreitz@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).