On 09.05.23 14:31, Hanna Czenczek wrote:
On 08.05.23 22:03, Eric Blake wrote:
Add some more strings that the user might send our way. In
particular, some of these additions include FIXME comments showing
where our parser doesn't quite behave the way we want.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
tests/unit/test-cutils.c | 226 +++++++++++++++++++++++++++++++++++++--
1 file changed, 215 insertions(+), 11 deletions(-)
I wonder: The plan is to have "1.5e+1k" be parsed as "1.5e" + endptr
== "+1k"; but "0x1p1" is not parsed at all (could be "0x1" + "p1"). Is
that fully intentional?
(Similarly, "1.1.k" is also not parsed at all, but the problem there
is not just two decimal points, but also that "1.1" would be an
invalid size in itself, so it really shouldn’t be parsed at all.)
I don’t think it matters to users, really, but I still wonder.
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index afae2ee5331..9fa6fb042e8 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
[...]
@@ -2875,6 +3056,20 @@ static void test_qemu_strtosz_trailing(void)
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
+
+ /* FIXME overflow in fraction is buggy */
+ str = "1.5E999";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpuint(res, ==, EiB /* FIXME EiB * 1.5 */);
So… I have no idea what happens here but this always fails with
“assertion failed (res == EiB): (1 == 1152921504606846976)”. But when I
replace the EiB by 1, it suddenly fails with “assertion failed (res ==
1): (1152921504606846976 == 1)” instead. Replacing the EiB by anything
but 1 also tells me that res is 1.
Now, here’s the kicker. I put an `fprintf(stderr, "res == %" PRIu64
"\n", res);` before this g_assert_cmpuint() (changed to (res, ==, 1))…
And it passes.
Sometimes I really want to change professions.
(Of note is that changing the g_assert() below into a g_assert_true()
also has g_assert_cmpuint(res, ==, 1) pass.)
+ g_assert(endptr == str + 9 /* FIXME + 4 */);
This is “correct” (i.e. it’s the value we’ll get right now, which is
the wrong one), but gcc complains that the array index is out of
bounds (well...), which breaks the build.
Oh, it also isn’t correct, I think it needs to be str + 8. As a bonus,
the compiler doesn’t complain then (for some reason…? it still seems
out of bounds).
(Otherwise, to get around the complaint, I used
g_assert_cmphex((uintptr_t)endptr, ==, (uintptr_t)str + 8). Which is
another thing, patch 1 explained to me that we shouldn’t use g_assert() :))
Hanna
