On 12.05.23 04:10, Eric Blake wrote:
It's hard to tweak code for consistency if I can't prove what will or
won't break from those tweaks.  Time to add unit tests for
qemu_strtod() and qemu_strtod_finite().

Among other things, I wrote a check whether we have C99 semantics for
strtod("0x1") (which MUST parse hex numbers) rather than C89 (which
must stop parsing at 'x').  These days, I suspect that is okay; but if
it fails CI checks, knowing the difference will help us decide what we
want to do about it.  Note that C2x, while not final at the time of
this patch, has been considering whether to make strtol("0b1") parse
as 1 with no slop instead of the C17 parse of 0 with slop "b1"; that
decision may also bleed over to strtod().  But for now, I didn't think
it worth adding unit tests on that front (to strtol or strtod) as
things may still change.

Likewise, there are plenty more corner cases of strtod proper that I
don't explicitly test here, but there are enough unit tests added here
that it covers all the branches reached in our wrappers.  In
particular, it demonstrates the difference on when *value is left
uninitialized, which an upcoming patch will normalize.

Signed-off-by: Eric Blake <ebl...@redhat.com>

---

v2: Added g_assert_false(signbit(res)) anywhere I used
g_assert_cmpfloat(res,==,0.0); add a test for strtod() hex parsing and
handling of junk after ERANGE, which is major enough that I dropped
R-b
---
  tests/unit/test-cutils.c | 510 +++++++++++++++++++++++++++++++++++++++
  1 file changed, 510 insertions(+)

diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index d3076c3fec1..1763839a157 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c

[...]

+static void test_qemu_strtod_erange_junk(void)
+{
+    const char *str;
+    const char *endptr;
+    int err;
+    double res;
+
+    /* EINVAL has priority over ERANGE */

By being placed here, this comment confused me a bit, because the first case does return ERANGE.  So I’d prefer it above the second case, where we actually expect EINVAL, but understand that’s a personal preference.  (Same for the _finite_ variant)

Reviewed-by: Hanna Czenczek <hre...@redhat.com>

+    str = "1e-999junk";
+    endptr = "somewhere";
+    res = 999;
+    err = qemu_strtod(str, &endptr, &res);
+    g_assert_cmpint(err, ==, -ERANGE);
+    g_assert_cmpfloat(res, <=, DBL_MIN);
+    g_assert_cmpfloat(res, >=, 0.0);
+    g_assert_false(signbit(res));
+    g_assert_true(endptr == str + 6);
+
+    endptr = "somewhere";
+    res = 999;
+    err = qemu_strtod(str, NULL, &res);
+    g_assert_cmpint(err, ==, -EINVAL);
+    g_assert_cmpfloat(res, ==, 0.0);
+    g_assert_false(signbit(res));
+}


Reply via email to