On Fri, May 19, 2023 at 05:26:12PM +0200, Hanna Czenczek wrote:
> On 12.05.23 04:10, 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 <[email protected]>
> >
> > ---
> >
> > v2: even more tests added, pad a string to avoid out-of-bounds
> > randomness [Hanna]
> > ---
> > tests/unit/test-cutils.c | 147 +++++++++++++++++++++++++++++++++++----
> > 1 file changed, 135 insertions(+), 12 deletions(-)
>
> The subject line appears as if it contained an ANSI escape sequence.
Yep, and I even flagged that in reply to the cover letter.
>
> > diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
> > index 1936c7b5795..7800caf9b0e 100644
> > --- a/tests/unit/test-cutils.c
> > +++ b/tests/unit/test-cutils.c
> > @@ -3162,7 +3162,12 @@ static void do_strtosz_full(const char *str,
> > qemu_strtosz_fn fn,
> > ret = fn(str, &endptr, &val);
> > g_assert_cmpint(ret, ==, exp_ptr_ret);
> > g_assert_cmpuint(val, ==, exp_ptr_val);
> > - g_assert_true(endptr == str + exp_ptr_offset);
> > + if (str) {
> > + g_assert_true(endptr == str + exp_ptr_offset);
> > + } else {
> > + g_assert_cmpint(exp_ptr_offset, ==, 0);
> > + g_assert_null(endptr);
> > + }
>
> This patch adds no new cases that call do_strtosz*() with a NULL str – did
> you intent for this to go into patch 12?
Oh, indeed - it was patch 12 that added do_strtosz(NULL, -EINVAL,
0xbaadf00d, 0); it's a shame the compiler doesn't complain about 'NULL
+ 0' as being an odd expression. Yes, I'll hoist this hunk to 12 for
v3...
>
> Regardless (with the subject fixed, though):
>
> Reviewed-by: Hanna Czenczek <[email protected]>
...while keeping your R-b.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
