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

[Hanna Czenczek <hreitz@redhat.com>]

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(-)
>
> 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 */);
> +
> +    res = 0xbaadf00d;
> +    err = qemu_strtosz(str, NULL, &res);
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert_cmphex(res, ==, 0xbaadf00d);

Got it now!

Our problem is that `endptr` is beyond the end of the string, precisely 
as gcc complains.  The data there is undefined, and depending on the 
value in the g_assert_cmpuint() (which is converted to strings for the 
potential error message) it sometimes is "endptr == str + 9" (the one in 
the g_assert()) and sometimes isn’t.

If it is "endptr == str + 9", then the 'e' is taken as a suffix, which 
makes `res == EiB`, and `endptr == "ndptr == str + 9"`.

If it isn’t, well, it might be anything, so there often is no valid 
suffix, making `res == 1`.

So the solution is to set `str = "1.5E999\0\0"`, so we don’t get out of 
bounds and know exactly what will be parsed.  Then, at str[8] there is 
no valid suffix (it’s \0), so `res == 1` and `endptr == str + 8`.  This 
will then lead to the qemu_strtosz(str, NULL, &res) below succeed, 
because, well, it’s a valid number.  I suppose it failed on your end 
because the out-of-bounds `str[9]` value was not '\0'.

That was a fun debugging session.

Hanna