Re: [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz

[Hanna Czenczek]

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 <eblake@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 */);
> +    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.

Hanna

> +
> +    res = 0xbaadf00d;
> +    err = qemu_strtosz(str, NULL, &res);
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert_cmphex(res, ==, 0xbaadf00d);
>   }