* [PATCH 01/11] test-cutils: Avoid g_assert in unit tests
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
@ 2023-05-08 20:03 ` Eric Blake
2023-05-08 20:03 ` [PATCH 02/11] test-cutils: Use g_assert_cmpuint where appropriate Eric Blake
` (10 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-08 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: hreitz
glib documentation[1] is clear: g_assert() should be avoided in unit
tests because it is ineffective if G_DISABLE_ASSERT is defined; unit
tests should stick to constructs based on g_assert_true() instead.
Note that since commit 262a69f428, we intentionally state that you
cannot define G_DISABLE_ASSERT that while building qemu; but our code
can be copied to other projects without that restriction, so we should
be consistent.
For most of the replacements in this patch, using g_assert_cmpstr()
would be a regression in quality - although it would helpfully display
the string contents of both pointers on test failure, here, we really
do care about pointer equality, not just string content equality. But
when a NULL pointer is expected, g_assert_null works fine.
[1] https://libsoup.org/glib/glib-Testing.html#g-assert
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/unit/test-cutils.c | 324 +++++++++++++++++++--------------------
1 file changed, 162 insertions(+), 162 deletions(-)
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 3c4f8754202..0202ac0d5b3 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -1,7 +1,7 @@
/*
* cutils.c unit-tests
*
- * Copyright (C) 2013 Red Hat Inc.
+ * Copyright Red Hat
*
* Authors:
* Eduardo Habkost <ehabkost@redhat.com>
@@ -40,7 +40,7 @@ static void test_parse_uint_null(void)
g_assert_cmpint(r, ==, -EINVAL);
g_assert_cmpint(i, ==, 0);
- g_assert(endptr == NULL);
+ g_assert_null(endptr);
}
static void test_parse_uint_empty(void)
@@ -55,7 +55,7 @@ static void test_parse_uint_empty(void)
g_assert_cmpint(r, ==, -EINVAL);
g_assert_cmpint(i, ==, 0);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_parse_uint_whitespace(void)
@@ -70,7 +70,7 @@ static void test_parse_uint_whitespace(void)
g_assert_cmpint(r, ==, -EINVAL);
g_assert_cmpint(i, ==, 0);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
@@ -86,7 +86,7 @@ static void test_parse_uint_invalid(void)
g_assert_cmpint(r, ==, -EINVAL);
g_assert_cmpint(i, ==, 0);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
@@ -102,7 +102,7 @@ static void test_parse_uint_trailing(void)
g_assert_cmpint(r, ==, 0);
g_assert_cmpint(i, ==, 123);
- g_assert(endptr == str + 3);
+ g_assert_true(endptr == str + 3);
}
static void test_parse_uint_correct(void)
@@ -117,7 +117,7 @@ static void test_parse_uint_correct(void)
g_assert_cmpint(r, ==, 0);
g_assert_cmpint(i, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_parse_uint_octal(void)
@@ -132,7 +132,7 @@ static void test_parse_uint_octal(void)
g_assert_cmpint(r, ==, 0);
g_assert_cmpint(i, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_parse_uint_decimal(void)
@@ -147,7 +147,7 @@ static void test_parse_uint_decimal(void)
g_assert_cmpint(r, ==, 0);
g_assert_cmpint(i, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
@@ -163,7 +163,7 @@ static void test_parse_uint_llong_max(void)
g_assert_cmpint(r, ==, 0);
g_assert_cmpint(i, ==, (unsigned long long)LLONG_MAX + 1);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
g_free(str);
}
@@ -180,7 +180,7 @@ static void test_parse_uint_overflow(void)
g_assert_cmpint(r, ==, -ERANGE);
g_assert_cmpint(i, ==, ULLONG_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_parse_uint_negative(void)
@@ -195,7 +195,7 @@ static void test_parse_uint_negative(void)
g_assert_cmpint(r, ==, -ERANGE);
g_assert_cmpint(i, ==, 0);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
@@ -235,7 +235,7 @@ static void test_qemu_strtoi_correct(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 12345);
- g_assert(endptr == str + 5);
+ g_assert_true(endptr == str + 5);
}
static void test_qemu_strtoi_null(void)
@@ -248,7 +248,7 @@ static void test_qemu_strtoi_null(void)
err = qemu_strtoi(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == NULL);
+ g_assert_null(endptr);
}
static void test_qemu_strtoi_empty(void)
@@ -262,7 +262,7 @@ static void test_qemu_strtoi_empty(void)
err = qemu_strtoi(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtoi_whitespace(void)
@@ -276,7 +276,7 @@ static void test_qemu_strtoi_whitespace(void)
err = qemu_strtoi(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtoi_invalid(void)
@@ -290,7 +290,7 @@ static void test_qemu_strtoi_invalid(void)
err = qemu_strtoi(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtoi_trailing(void)
@@ -305,7 +305,7 @@ static void test_qemu_strtoi_trailing(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 123);
- g_assert(endptr == str + 3);
+ g_assert_true(endptr == str + 3);
}
static void test_qemu_strtoi_octal(void)
@@ -320,7 +320,7 @@ static void test_qemu_strtoi_octal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
res = 999;
endptr = &f;
@@ -328,7 +328,7 @@ static void test_qemu_strtoi_octal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoi_decimal(void)
@@ -343,7 +343,7 @@ static void test_qemu_strtoi_decimal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "123";
res = 999;
@@ -352,7 +352,7 @@ static void test_qemu_strtoi_decimal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoi_hex(void)
@@ -367,7 +367,7 @@ static void test_qemu_strtoi_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0x123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "0x123";
res = 999;
@@ -376,7 +376,7 @@ static void test_qemu_strtoi_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0x123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "0x";
res = 999;
@@ -385,7 +385,7 @@ static void test_qemu_strtoi_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0);
- g_assert(endptr == str + 1);
+ g_assert_true(endptr == str + 1);
}
static void test_qemu_strtoi_max(void)
@@ -400,7 +400,7 @@ static void test_qemu_strtoi_max(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, INT_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
g_free(str);
}
@@ -416,7 +416,7 @@ static void test_qemu_strtoi_overflow(void)
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpint(res, ==, INT_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
g_free(str);
}
@@ -432,7 +432,7 @@ static void test_qemu_strtoi_underflow(void)
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpint(res, ==, INT_MIN);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
g_free(str);
}
@@ -448,7 +448,7 @@ static void test_qemu_strtoi_negative(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, -321);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoi_full_correct(void)
@@ -473,7 +473,7 @@ static void test_qemu_strtoi_full_null(void)
err = qemu_strtoi(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == NULL);
+ g_assert_null(endptr);
}
static void test_qemu_strtoi_full_empty(void)
@@ -535,7 +535,7 @@ static void test_qemu_strtoui_correct(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 12345);
- g_assert(endptr == str + 5);
+ g_assert_true(endptr == str + 5);
}
static void test_qemu_strtoui_null(void)
@@ -548,7 +548,7 @@ static void test_qemu_strtoui_null(void)
err = qemu_strtoui(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == NULL);
+ g_assert_null(endptr);
}
static void test_qemu_strtoui_empty(void)
@@ -562,7 +562,7 @@ static void test_qemu_strtoui_empty(void)
err = qemu_strtoui(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtoui_whitespace(void)
@@ -576,7 +576,7 @@ static void test_qemu_strtoui_whitespace(void)
err = qemu_strtoui(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtoui_invalid(void)
@@ -590,7 +590,7 @@ static void test_qemu_strtoui_invalid(void)
err = qemu_strtoui(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtoui_trailing(void)
@@ -605,7 +605,7 @@ static void test_qemu_strtoui_trailing(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 123);
- g_assert(endptr == str + 3);
+ g_assert_true(endptr == str + 3);
}
static void test_qemu_strtoui_octal(void)
@@ -620,7 +620,7 @@ static void test_qemu_strtoui_octal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
res = 999;
endptr = &f;
@@ -628,7 +628,7 @@ static void test_qemu_strtoui_octal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoui_decimal(void)
@@ -643,7 +643,7 @@ static void test_qemu_strtoui_decimal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "123";
res = 999;
@@ -652,7 +652,7 @@ static void test_qemu_strtoui_decimal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoui_hex(void)
@@ -667,7 +667,7 @@ static void test_qemu_strtoui_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmphex(res, ==, 0x123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "0x123";
res = 999;
@@ -676,7 +676,7 @@ static void test_qemu_strtoui_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmphex(res, ==, 0x123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "0x";
res = 999;
@@ -685,7 +685,7 @@ static void test_qemu_strtoui_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmphex(res, ==, 0);
- g_assert(endptr == str + 1);
+ g_assert_true(endptr == str + 1);
}
static void test_qemu_strtoui_max(void)
@@ -700,7 +700,7 @@ static void test_qemu_strtoui_max(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmphex(res, ==, UINT_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
g_free(str);
}
@@ -716,7 +716,7 @@ static void test_qemu_strtoui_overflow(void)
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmphex(res, ==, UINT_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
g_free(str);
}
@@ -732,7 +732,7 @@ static void test_qemu_strtoui_underflow(void)
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpuint(res, ==, (unsigned int)-1);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
g_free(str);
}
@@ -748,7 +748,7 @@ static void test_qemu_strtoui_negative(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, (unsigned int)-321);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoui_full_correct(void)
@@ -830,7 +830,7 @@ static void test_qemu_strtol_correct(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 12345);
- g_assert(endptr == str + 5);
+ g_assert_true(endptr == str + 5);
}
static void test_qemu_strtol_null(void)
@@ -843,7 +843,7 @@ static void test_qemu_strtol_null(void)
err = qemu_strtol(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == NULL);
+ g_assert_null(endptr);
}
static void test_qemu_strtol_empty(void)
@@ -857,7 +857,7 @@ static void test_qemu_strtol_empty(void)
err = qemu_strtol(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtol_whitespace(void)
@@ -871,7 +871,7 @@ static void test_qemu_strtol_whitespace(void)
err = qemu_strtol(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtol_invalid(void)
@@ -885,7 +885,7 @@ static void test_qemu_strtol_invalid(void)
err = qemu_strtol(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtol_trailing(void)
@@ -900,7 +900,7 @@ static void test_qemu_strtol_trailing(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 123);
- g_assert(endptr == str + 3);
+ g_assert_true(endptr == str + 3);
}
static void test_qemu_strtol_octal(void)
@@ -915,7 +915,7 @@ static void test_qemu_strtol_octal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
res = 999;
endptr = &f;
@@ -923,7 +923,7 @@ static void test_qemu_strtol_octal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtol_decimal(void)
@@ -938,7 +938,7 @@ static void test_qemu_strtol_decimal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "123";
res = 999;
@@ -947,7 +947,7 @@ static void test_qemu_strtol_decimal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtol_hex(void)
@@ -962,7 +962,7 @@ static void test_qemu_strtol_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0x123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "0x123";
res = 999;
@@ -971,7 +971,7 @@ static void test_qemu_strtol_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0x123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "0x";
res = 999;
@@ -980,7 +980,7 @@ static void test_qemu_strtol_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0);
- g_assert(endptr == str + 1);
+ g_assert_true(endptr == str + 1);
}
static void test_qemu_strtol_max(void)
@@ -995,7 +995,7 @@ static void test_qemu_strtol_max(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, LONG_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
g_free(str);
}
@@ -1011,7 +1011,7 @@ static void test_qemu_strtol_overflow(void)
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpint(res, ==, LONG_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtol_underflow(void)
@@ -1026,7 +1026,7 @@ static void test_qemu_strtol_underflow(void)
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpint(res, ==, LONG_MIN);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtol_negative(void)
@@ -1041,7 +1041,7 @@ static void test_qemu_strtol_negative(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, -321);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtol_full_correct(void)
@@ -1066,7 +1066,7 @@ static void test_qemu_strtol_full_null(void)
err = qemu_strtol(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == NULL);
+ g_assert_null(endptr);
}
static void test_qemu_strtol_full_empty(void)
@@ -1128,7 +1128,7 @@ static void test_qemu_strtoul_correct(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 12345);
- g_assert(endptr == str + 5);
+ g_assert_true(endptr == str + 5);
}
static void test_qemu_strtoul_null(void)
@@ -1141,7 +1141,7 @@ static void test_qemu_strtoul_null(void)
err = qemu_strtoul(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == NULL);
+ g_assert_null(endptr);
}
static void test_qemu_strtoul_empty(void)
@@ -1155,7 +1155,7 @@ static void test_qemu_strtoul_empty(void)
err = qemu_strtoul(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtoul_whitespace(void)
@@ -1169,7 +1169,7 @@ static void test_qemu_strtoul_whitespace(void)
err = qemu_strtoul(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtoul_invalid(void)
@@ -1183,7 +1183,7 @@ static void test_qemu_strtoul_invalid(void)
err = qemu_strtoul(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtoul_trailing(void)
@@ -1198,7 +1198,7 @@ static void test_qemu_strtoul_trailing(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 123);
- g_assert(endptr == str + 3);
+ g_assert_true(endptr == str + 3);
}
static void test_qemu_strtoul_octal(void)
@@ -1213,7 +1213,7 @@ static void test_qemu_strtoul_octal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
res = 999;
endptr = &f;
@@ -1221,7 +1221,7 @@ static void test_qemu_strtoul_octal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoul_decimal(void)
@@ -1236,7 +1236,7 @@ static void test_qemu_strtoul_decimal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "123";
res = 999;
@@ -1245,7 +1245,7 @@ static void test_qemu_strtoul_decimal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoul_hex(void)
@@ -1260,7 +1260,7 @@ static void test_qemu_strtoul_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmphex(res, ==, 0x123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "0x123";
res = 999;
@@ -1269,7 +1269,7 @@ static void test_qemu_strtoul_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmphex(res, ==, 0x123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "0x";
res = 999;
@@ -1278,7 +1278,7 @@ static void test_qemu_strtoul_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmphex(res, ==, 0);
- g_assert(endptr == str + 1);
+ g_assert_true(endptr == str + 1);
}
static void test_qemu_strtoul_max(void)
@@ -1293,7 +1293,7 @@ static void test_qemu_strtoul_max(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmphex(res, ==, ULONG_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
g_free(str);
}
@@ -1309,7 +1309,7 @@ static void test_qemu_strtoul_overflow(void)
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmphex(res, ==, ULONG_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoul_underflow(void)
@@ -1324,7 +1324,7 @@ static void test_qemu_strtoul_underflow(void)
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpuint(res, ==, -1ul);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoul_negative(void)
@@ -1339,7 +1339,7 @@ static void test_qemu_strtoul_negative(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, -321ul);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoul_full_correct(void)
@@ -1421,7 +1421,7 @@ static void test_qemu_strtoi64_correct(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 12345);
- g_assert(endptr == str + 5);
+ g_assert_true(endptr == str + 5);
}
static void test_qemu_strtoi64_null(void)
@@ -1434,7 +1434,7 @@ static void test_qemu_strtoi64_null(void)
err = qemu_strtoi64(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == NULL);
+ g_assert_null(endptr);
}
static void test_qemu_strtoi64_empty(void)
@@ -1448,7 +1448,7 @@ static void test_qemu_strtoi64_empty(void)
err = qemu_strtoi64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtoi64_whitespace(void)
@@ -1462,7 +1462,7 @@ static void test_qemu_strtoi64_whitespace(void)
err = qemu_strtoi64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtoi64_invalid(void)
@@ -1476,7 +1476,7 @@ static void test_qemu_strtoi64_invalid(void)
err = qemu_strtoi64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtoi64_trailing(void)
@@ -1491,7 +1491,7 @@ static void test_qemu_strtoi64_trailing(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 123);
- g_assert(endptr == str + 3);
+ g_assert_true(endptr == str + 3);
}
static void test_qemu_strtoi64_octal(void)
@@ -1506,7 +1506,7 @@ static void test_qemu_strtoi64_octal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
endptr = &f;
res = 999;
@@ -1514,7 +1514,7 @@ static void test_qemu_strtoi64_octal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoi64_decimal(void)
@@ -1529,7 +1529,7 @@ static void test_qemu_strtoi64_decimal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "123";
endptr = &f;
@@ -1538,7 +1538,7 @@ static void test_qemu_strtoi64_decimal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoi64_hex(void)
@@ -1553,7 +1553,7 @@ static void test_qemu_strtoi64_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0x123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "0x123";
endptr = &f;
@@ -1562,7 +1562,7 @@ static void test_qemu_strtoi64_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0x123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "0x";
endptr = &f;
@@ -1571,7 +1571,7 @@ static void test_qemu_strtoi64_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0);
- g_assert(endptr == str + 1);
+ g_assert_true(endptr == str + 1);
}
static void test_qemu_strtoi64_max(void)
@@ -1586,7 +1586,7 @@ static void test_qemu_strtoi64_max(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, LLONG_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
g_free(str);
}
@@ -1602,7 +1602,7 @@ static void test_qemu_strtoi64_overflow(void)
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpint(res, ==, LLONG_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoi64_underflow(void)
@@ -1617,7 +1617,7 @@ static void test_qemu_strtoi64_underflow(void)
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpint(res, ==, LLONG_MIN);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoi64_negative(void)
@@ -1632,7 +1632,7 @@ static void test_qemu_strtoi64_negative(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, -321);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtoi64_full_correct(void)
@@ -1717,7 +1717,7 @@ static void test_qemu_strtou64_correct(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 12345);
- g_assert(endptr == str + 5);
+ g_assert_true(endptr == str + 5);
}
static void test_qemu_strtou64_null(void)
@@ -1730,7 +1730,7 @@ static void test_qemu_strtou64_null(void)
err = qemu_strtou64(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == NULL);
+ g_assert_null(endptr);
}
static void test_qemu_strtou64_empty(void)
@@ -1744,7 +1744,7 @@ static void test_qemu_strtou64_empty(void)
err = qemu_strtou64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtou64_whitespace(void)
@@ -1758,7 +1758,7 @@ static void test_qemu_strtou64_whitespace(void)
err = qemu_strtou64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtou64_invalid(void)
@@ -1772,7 +1772,7 @@ static void test_qemu_strtou64_invalid(void)
err = qemu_strtou64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtou64_trailing(void)
@@ -1787,7 +1787,7 @@ static void test_qemu_strtou64_trailing(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 123);
- g_assert(endptr == str + 3);
+ g_assert_true(endptr == str + 3);
}
static void test_qemu_strtou64_octal(void)
@@ -1802,7 +1802,7 @@ static void test_qemu_strtou64_octal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
endptr = &f;
res = 999;
@@ -1810,7 +1810,7 @@ static void test_qemu_strtou64_octal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 0123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtou64_decimal(void)
@@ -1825,7 +1825,7 @@ static void test_qemu_strtou64_decimal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "123";
endptr = &f;
@@ -1834,7 +1834,7 @@ static void test_qemu_strtou64_decimal(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtou64_hex(void)
@@ -1849,7 +1849,7 @@ static void test_qemu_strtou64_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmphex(res, ==, 0x123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "0x123";
endptr = &f;
@@ -1858,7 +1858,7 @@ static void test_qemu_strtou64_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmphex(res, ==, 0x123);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
str = "0x";
endptr = &f;
@@ -1867,7 +1867,7 @@ static void test_qemu_strtou64_hex(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmphex(res, ==, 0);
- g_assert(endptr == str + 1);
+ g_assert_true(endptr == str + 1);
}
static void test_qemu_strtou64_max(void)
@@ -1882,7 +1882,7 @@ static void test_qemu_strtou64_max(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmphex(res, ==, ULLONG_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
g_free(str);
}
@@ -1898,7 +1898,7 @@ static void test_qemu_strtou64_overflow(void)
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmphex(res, ==, ULLONG_MAX);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtou64_underflow(void)
@@ -1913,7 +1913,7 @@ static void test_qemu_strtou64_underflow(void)
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmphex(res, ==, -1ull);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtou64_negative(void)
@@ -1928,7 +1928,7 @@ static void test_qemu_strtou64_negative(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, -321ull);
- g_assert(endptr == str + strlen(str));
+ g_assert_true(endptr == str + strlen(str));
}
static void test_qemu_strtou64_full_correct(void)
@@ -2013,7 +2013,7 @@ static void test_qemu_strtosz_simple(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0);
- g_assert(endptr == str + 1);
+ g_assert_true(endptr == str + 1);
/* Leading 0 gives decimal results, not octal */
str = "08";
@@ -2022,7 +2022,7 @@ static void test_qemu_strtosz_simple(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 8);
- g_assert(endptr == str + 2);
+ g_assert_true(endptr == str + 2);
/* Leading space is ignored */
str = " 12345";
@@ -2031,7 +2031,7 @@ static void test_qemu_strtosz_simple(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 12345);
- g_assert(endptr == str + 6);
+ g_assert_true(endptr == str + 6);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
@@ -2044,7 +2044,7 @@ static void test_qemu_strtosz_simple(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0x1fffffffffffff);
- g_assert(endptr == str + 16);
+ g_assert_true(endptr == str + 16);
str = "9007199254740992"; /* 2^53 */
endptr = str;
@@ -2052,7 +2052,7 @@ static void test_qemu_strtosz_simple(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0x20000000000000);
- g_assert(endptr == str + 16);
+ g_assert_true(endptr == str + 16);
str = "9007199254740993"; /* 2^53+1 */
endptr = str;
@@ -2060,7 +2060,7 @@ static void test_qemu_strtosz_simple(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0x20000000000001);
- g_assert(endptr == str + 16);
+ g_assert_true(endptr == str + 16);
str = "18446744073709549568"; /* 0xfffffffffffff800 (53 msbs set) */
endptr = str;
@@ -2068,7 +2068,7 @@ static void test_qemu_strtosz_simple(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0xfffffffffffff800);
- g_assert(endptr == str + 20);
+ g_assert_true(endptr == str + 20);
str = "18446744073709550591"; /* 0xfffffffffffffbff */
endptr = str;
@@ -2076,7 +2076,7 @@ static void test_qemu_strtosz_simple(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0xfffffffffffffbff);
- g_assert(endptr == str + 20);
+ g_assert_true(endptr == str + 20);
str = "18446744073709551615"; /* 0xffffffffffffffff */
endptr = str;
@@ -2084,7 +2084,7 @@ static void test_qemu_strtosz_simple(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0xffffffffffffffff);
- g_assert(endptr == str + 20);
+ g_assert_true(endptr == str + 20);
}
static void test_qemu_strtosz_hex(void)
@@ -2100,7 +2100,7 @@ static void test_qemu_strtosz_hex(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0);
- g_assert(endptr == str + 3);
+ g_assert_true(endptr == str + 3);
str = "0xab";
endptr = str;
@@ -2108,7 +2108,7 @@ static void test_qemu_strtosz_hex(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 171);
- g_assert(endptr == str + 4);
+ g_assert_true(endptr == str + 4);
str = "0xae";
endptr = str;
@@ -2116,7 +2116,7 @@ static void test_qemu_strtosz_hex(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 174);
- g_assert(endptr == str + 4);
+ g_assert_true(endptr == str + 4);
}
static void test_qemu_strtosz_units(void)
@@ -2139,56 +2139,56 @@ static void test_qemu_strtosz_units(void)
err = qemu_strtosz_MiB(none, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, MiB);
- g_assert(endptr == none + 1);
+ g_assert_true(endptr == none + 1);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(b, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 1);
- g_assert(endptr == b + 2);
+ g_assert_true(endptr == b + 2);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(k, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, KiB);
- g_assert(endptr == k + 2);
+ g_assert_true(endptr == k + 2);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(m, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, MiB);
- g_assert(endptr == m + 2);
+ g_assert_true(endptr == m + 2);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(g, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, GiB);
- g_assert(endptr == g + 2);
+ g_assert_true(endptr == g + 2);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(t, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, TiB);
- g_assert(endptr == t + 2);
+ g_assert_true(endptr == t + 2);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(p, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, PiB);
- g_assert(endptr == p + 2);
+ g_assert_true(endptr == p + 2);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(e, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, EiB);
- g_assert(endptr == e + 2);
+ g_assert_true(endptr == e + 2);
}
static void test_qemu_strtosz_float(void)
@@ -2204,7 +2204,7 @@ static void test_qemu_strtosz_float(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, EiB / 2);
- g_assert(endptr == str + 4);
+ g_assert_true(endptr == str + 4);
/* For convenience, a fraction of 0 is tolerated even on bytes */
str = "1.0B";
@@ -2213,7 +2213,7 @@ static void test_qemu_strtosz_float(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 1);
- g_assert(endptr == str + 4);
+ g_assert_true(endptr == str + 4);
/* An empty fraction is tolerated */
str = "1.k";
@@ -2222,7 +2222,7 @@ static void test_qemu_strtosz_float(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 1024);
- g_assert(endptr == str + 3);
+ g_assert_true(endptr == str + 3);
/* For convenience, we permit values that are not byte-exact */
str = "12.345M";
@@ -2231,7 +2231,7 @@ static void test_qemu_strtosz_float(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB + 0.5));
- g_assert(endptr == str + 7);
+ g_assert_true(endptr == str + 7);
}
static void test_qemu_strtosz_invalid(void)
@@ -2246,35 +2246,35 @@ static void test_qemu_strtosz_invalid(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
str = " \t ";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
str = "crap";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
str = "inf";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
str = "NaN";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
/* Fractional values require scale larger than bytes */
str = "1.1B";
@@ -2282,14 +2282,14 @@ static void test_qemu_strtosz_invalid(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
str = "1.1";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
/* No floating point exponents */
str = "1.5e1k";
@@ -2297,14 +2297,14 @@ static void test_qemu_strtosz_invalid(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
str = "1.5E+0k";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
/* No hex fractions */
str = "0x1.8k";
@@ -2312,7 +2312,7 @@ static void test_qemu_strtosz_invalid(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
/* No suffixes */
str = "0x18M";
@@ -2320,7 +2320,7 @@ static void test_qemu_strtosz_invalid(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
/* No negative values */
str = "-0";
@@ -2328,14 +2328,14 @@ static void test_qemu_strtosz_invalid(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
str = "-1";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str);
+ g_assert_true(endptr == str);
}
static void test_qemu_strtosz_trailing(void)
@@ -2351,7 +2351,7 @@ static void test_qemu_strtosz_trailing(void)
err = qemu_strtosz_MiB(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 123 * MiB);
- g_assert(endptr == str + 3);
+ g_assert_true(endptr == str + 3);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
@@ -2364,7 +2364,7 @@ static void test_qemu_strtosz_trailing(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 1024);
- g_assert(endptr == str + 2);
+ g_assert_true(endptr == str + 2);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
@@ -2377,7 +2377,7 @@ static void test_qemu_strtosz_trailing(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0);
- g_assert(endptr == str + 1);
+ g_assert_true(endptr == str + 1);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
@@ -2390,7 +2390,7 @@ static void test_qemu_strtosz_trailing(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 0);
- g_assert(endptr == str + 2);
+ g_assert_true(endptr == str + 2);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
@@ -2403,7 +2403,7 @@ static void test_qemu_strtosz_trailing(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 123);
- g_assert(endptr == str + 3);
+ g_assert_true(endptr == str + 3);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
@@ -2423,14 +2423,14 @@ static void test_qemu_strtosz_erange(void)
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str + 20);
+ g_assert_true(endptr == str + 20);
str = "20E";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpint(res, ==, 0xbaadf00d);
- g_assert(endptr == str + 3);
+ g_assert_true(endptr == str + 3);
}
static void test_qemu_strtosz_metric(void)
@@ -2446,7 +2446,7 @@ static void test_qemu_strtosz_metric(void)
err = qemu_strtosz_metric(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 12345000);
- g_assert(endptr == str + 6);
+ g_assert_true(endptr == str + 6);
str = "12.345M";
endptr = str;
@@ -2454,7 +2454,7 @@ static void test_qemu_strtosz_metric(void)
err = qemu_strtosz_metric(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpint(res, ==, 12345000);
- g_assert(endptr == str + 7);
+ g_assert_true(endptr == str + 7);
}
static void test_freq_to_str(void)
--
2.40.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 02/11] test-cutils: Use g_assert_cmpuint where appropriate
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
2023-05-08 20:03 ` [PATCH 01/11] test-cutils: Avoid g_assert in unit tests Eric Blake
@ 2023-05-08 20:03 ` Eric Blake
2023-05-08 20:03 ` [PATCH 03/11] test-cutils: Test integral qemu_strto* value on failures Eric Blake
` (9 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-08 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: hreitz
When debugging test failures, seeing unsigned values as large positive
values rather than negative values matters (assuming that the bug in
glib 2.76 [1] where g_assert_cmpuint displays signed instead of
unsigned values will eventually be fixed). No impact when the test is
passing, but using a consistent style will matter more in upcoming
test additions. Also, some tests are better with cmphex.
While at it, fix some spacing and minor typing issues spotted nearby.
[1] https://gitlab.gnome.org/GNOME/glib/-/issues/2997
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/unit/test-cutils.c | 148 +++++++++++++++++++--------------------
1 file changed, 74 insertions(+), 74 deletions(-)
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 0202ac0d5b3..38bd3990207 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -39,7 +39,7 @@ static void test_parse_uint_null(void)
r = parse_uint(NULL, &i, &endptr, 0);
g_assert_cmpint(r, ==, -EINVAL);
- g_assert_cmpint(i, ==, 0);
+ g_assert_cmpuint(i, ==, 0);
g_assert_null(endptr);
}
@@ -54,7 +54,7 @@ static void test_parse_uint_empty(void)
r = parse_uint(str, &i, &endptr, 0);
g_assert_cmpint(r, ==, -EINVAL);
- g_assert_cmpint(i, ==, 0);
+ g_assert_cmpuint(i, ==, 0);
g_assert_true(endptr == str);
}
@@ -69,7 +69,7 @@ static void test_parse_uint_whitespace(void)
r = parse_uint(str, &i, &endptr, 0);
g_assert_cmpint(r, ==, -EINVAL);
- g_assert_cmpint(i, ==, 0);
+ g_assert_cmpuint(i, ==, 0);
g_assert_true(endptr == str);
}
@@ -85,7 +85,7 @@ static void test_parse_uint_invalid(void)
r = parse_uint(str, &i, &endptr, 0);
g_assert_cmpint(r, ==, -EINVAL);
- g_assert_cmpint(i, ==, 0);
+ g_assert_cmpuint(i, ==, 0);
g_assert_true(endptr == str);
}
@@ -101,7 +101,7 @@ static void test_parse_uint_trailing(void)
r = parse_uint(str, &i, &endptr, 0);
g_assert_cmpint(r, ==, 0);
- g_assert_cmpint(i, ==, 123);
+ g_assert_cmpuint(i, ==, 123);
g_assert_true(endptr == str + 3);
}
@@ -116,7 +116,7 @@ static void test_parse_uint_correct(void)
r = parse_uint(str, &i, &endptr, 0);
g_assert_cmpint(r, ==, 0);
- g_assert_cmpint(i, ==, 123);
+ g_assert_cmpuint(i, ==, 123);
g_assert_true(endptr == str + strlen(str));
}
@@ -131,7 +131,7 @@ static void test_parse_uint_octal(void)
r = parse_uint(str, &i, &endptr, 0);
g_assert_cmpint(r, ==, 0);
- g_assert_cmpint(i, ==, 0123);
+ g_assert_cmpuint(i, ==, 0123);
g_assert_true(endptr == str + strlen(str));
}
@@ -146,7 +146,7 @@ static void test_parse_uint_decimal(void)
r = parse_uint(str, &i, &endptr, 10);
g_assert_cmpint(r, ==, 0);
- g_assert_cmpint(i, ==, 123);
+ g_assert_cmpuint(i, ==, 123);
g_assert_true(endptr == str + strlen(str));
}
@@ -162,7 +162,7 @@ static void test_parse_uint_llong_max(void)
r = parse_uint(str, &i, &endptr, 0);
g_assert_cmpint(r, ==, 0);
- g_assert_cmpint(i, ==, (unsigned long long)LLONG_MAX + 1);
+ g_assert_cmpuint(i, ==, (unsigned long long)LLONG_MAX + 1);
g_assert_true(endptr == str + strlen(str));
g_free(str);
@@ -179,7 +179,7 @@ static void test_parse_uint_overflow(void)
r = parse_uint(str, &i, &endptr, 0);
g_assert_cmpint(r, ==, -ERANGE);
- g_assert_cmpint(i, ==, ULLONG_MAX);
+ g_assert_cmpuint(i, ==, ULLONG_MAX);
g_assert_true(endptr == str + strlen(str));
}
@@ -194,7 +194,7 @@ static void test_parse_uint_negative(void)
r = parse_uint(str, &i, &endptr, 0);
g_assert_cmpint(r, ==, -ERANGE);
- g_assert_cmpint(i, ==, 0);
+ g_assert_cmpuint(i, ==, 0);
g_assert_true(endptr == str + strlen(str));
}
@@ -208,7 +208,7 @@ static void test_parse_uint_full_trailing(void)
r = parse_uint_full(str, &i, 0);
g_assert_cmpint(r, ==, -EINVAL);
- g_assert_cmpint(i, ==, 0);
+ g_assert_cmpuint(i, ==, 0);
}
static void test_parse_uint_full_correct(void)
@@ -220,7 +220,7 @@ static void test_parse_uint_full_correct(void)
r = parse_uint_full(str, &i, 0);
g_assert_cmpint(r, ==, 0);
- g_assert_cmpint(i, ==, 123);
+ g_assert_cmpuint(i, ==, 123);
}
static void test_qemu_strtoi_correct(void)
@@ -428,7 +428,7 @@ static void test_qemu_strtoi_underflow(void)
int res = 999;
int err;
- err = qemu_strtoi(str, &endptr, 0, &res);
+ err = qemu_strtoi(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpint(res, ==, INT_MIN);
@@ -479,10 +479,10 @@ static void test_qemu_strtoi_full_null(void)
static void test_qemu_strtoi_full_empty(void)
{
const char *str = "";
- int res = 999L;
+ int res = 999;
int err;
- err = qemu_strtoi(str, NULL, 0, &res);
+ err = qemu_strtoi(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
}
@@ -728,7 +728,7 @@ static void test_qemu_strtoui_underflow(void)
unsigned int res = 999;
int err;
- err = qemu_strtoui(str, &endptr, 0, &res);
+ err = qemu_strtoui(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpuint(res, ==, (unsigned int)-1);
@@ -1022,7 +1022,7 @@ static void test_qemu_strtol_underflow(void)
long res = 999;
int err;
- err = qemu_strtol(str, &endptr, 0, &res);
+ err = qemu_strtol(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpint(res, ==, LONG_MIN);
@@ -1075,7 +1075,7 @@ static void test_qemu_strtol_full_empty(void)
long res = 999L;
int err;
- err = qemu_strtol(str, NULL, 0, &res);
+ err = qemu_strtol(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
}
@@ -1320,7 +1320,7 @@ static void test_qemu_strtoul_underflow(void)
unsigned long res = 999;
int err;
- err = qemu_strtoul(str, &endptr, 0, &res);
+ err = qemu_strtoul(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpuint(res, ==, -1ul);
@@ -1613,7 +1613,7 @@ static void test_qemu_strtoi64_underflow(void)
int64_t res = 999;
int err;
- err = qemu_strtoi64(str, &endptr, 0, &res);
+ err = qemu_strtoi64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmpint(res, ==, LLONG_MIN);
@@ -1909,7 +1909,7 @@ static void test_qemu_strtou64_underflow(void)
uint64_t res = 999;
int err;
- err = qemu_strtou64(str, &endptr, 0, &res);
+ err = qemu_strtou64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -ERANGE);
g_assert_cmphex(res, ==, -1ull);
@@ -2012,7 +2012,7 @@ static void test_qemu_strtosz_simple(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 0);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str + 1);
/* Leading 0 gives decimal results, not octal */
@@ -2021,7 +2021,7 @@ static void test_qemu_strtosz_simple(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 8);
+ g_assert_cmpuint(res, ==, 8);
g_assert_true(endptr == str + 2);
/* Leading space is ignored */
@@ -2030,20 +2030,20 @@ static void test_qemu_strtosz_simple(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 12345);
+ g_assert_cmpuint(res, ==, 12345);
g_assert_true(endptr == str + 6);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 12345);
+ g_assert_cmpuint(res, ==, 12345);
str = "9007199254740991"; /* 2^53-1 */
endptr = str;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 0x1fffffffffffff);
+ g_assert_cmphex(res, ==, 0x1fffffffffffffULL);
g_assert_true(endptr == str + 16);
str = "9007199254740992"; /* 2^53 */
@@ -2051,7 +2051,7 @@ static void test_qemu_strtosz_simple(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 0x20000000000000);
+ g_assert_cmphex(res, ==, 0x20000000000000ULL);
g_assert_true(endptr == str + 16);
str = "9007199254740993"; /* 2^53+1 */
@@ -2059,7 +2059,7 @@ static void test_qemu_strtosz_simple(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 0x20000000000001);
+ g_assert_cmphex(res, ==, 0x20000000000001ULL);
g_assert_true(endptr == str + 16);
str = "18446744073709549568"; /* 0xfffffffffffff800 (53 msbs set) */
@@ -2067,7 +2067,7 @@ static void test_qemu_strtosz_simple(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 0xfffffffffffff800);
+ g_assert_cmphex(res, ==, 0xfffffffffffff800ULL);
g_assert_true(endptr == str + 20);
str = "18446744073709550591"; /* 0xfffffffffffffbff */
@@ -2075,7 +2075,7 @@ static void test_qemu_strtosz_simple(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 0xfffffffffffffbff);
+ g_assert_cmphex(res, ==, 0xfffffffffffffbffULL);
g_assert_true(endptr == str + 20);
str = "18446744073709551615"; /* 0xffffffffffffffff */
@@ -2083,7 +2083,7 @@ static void test_qemu_strtosz_simple(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 0xffffffffffffffff);
+ g_assert_cmphex(res, ==, 0xffffffffffffffffULL);
g_assert_true(endptr == str + 20);
}
@@ -2099,7 +2099,7 @@ static void test_qemu_strtosz_hex(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 0);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str + 3);
str = "0xab";
@@ -2107,7 +2107,7 @@ static void test_qemu_strtosz_hex(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 171);
+ g_assert_cmpuint(res, ==, 171);
g_assert_true(endptr == str + 4);
str = "0xae";
@@ -2115,7 +2115,7 @@ static void test_qemu_strtosz_hex(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 174);
+ g_assert_cmpuint(res, ==, 174);
g_assert_true(endptr == str + 4);
}
@@ -2138,56 +2138,56 @@ static void test_qemu_strtosz_units(void)
res = 0xbaadf00d;
err = qemu_strtosz_MiB(none, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, MiB);
+ g_assert_cmpuint(res, ==, MiB);
g_assert_true(endptr == none + 1);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(b, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 1);
+ g_assert_cmpuint(res, ==, 1);
g_assert_true(endptr == b + 2);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(k, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, KiB);
+ g_assert_cmpuint(res, ==, KiB);
g_assert_true(endptr == k + 2);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(m, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, MiB);
+ g_assert_cmpuint(res, ==, MiB);
g_assert_true(endptr == m + 2);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(g, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, GiB);
+ g_assert_cmpuint(res, ==, GiB);
g_assert_true(endptr == g + 2);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(t, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, TiB);
+ g_assert_cmpuint(res, ==, TiB);
g_assert_true(endptr == t + 2);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(p, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, PiB);
+ g_assert_cmpuint(res, ==, PiB);
g_assert_true(endptr == p + 2);
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(e, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, EiB);
+ g_assert_cmpuint(res, ==, EiB);
g_assert_true(endptr == e + 2);
}
@@ -2203,7 +2203,7 @@ static void test_qemu_strtosz_float(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, EiB / 2);
+ g_assert_cmpuint(res, ==, EiB / 2);
g_assert_true(endptr == str + 4);
/* For convenience, a fraction of 0 is tolerated even on bytes */
@@ -2212,7 +2212,7 @@ static void test_qemu_strtosz_float(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 1);
+ g_assert_cmpuint(res, ==, 1);
g_assert_true(endptr == str + 4);
/* An empty fraction is tolerated */
@@ -2221,7 +2221,7 @@ static void test_qemu_strtosz_float(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 1024);
+ g_assert_cmpuint(res, ==, 1024);
g_assert_true(endptr == str + 3);
/* For convenience, we permit values that are not byte-exact */
@@ -2230,7 +2230,7 @@ static void test_qemu_strtosz_float(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB + 0.5));
+ g_assert_cmpuint(res, ==, (uint64_t) (12.345 * MiB + 0.5));
g_assert_true(endptr == str + 7);
}
@@ -2245,35 +2245,35 @@ static void test_qemu_strtosz_invalid(void)
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
str = " \t ";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
str = "crap";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
str = "inf";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
str = "NaN";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
/* Fractional values require scale larger than bytes */
@@ -2281,14 +2281,14 @@ static void test_qemu_strtosz_invalid(void)
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
str = "1.1";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
/* No floating point exponents */
@@ -2296,14 +2296,14 @@ static void test_qemu_strtosz_invalid(void)
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
str = "1.5E+0k";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
/* No hex fractions */
@@ -2311,7 +2311,7 @@ static void test_qemu_strtosz_invalid(void)
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
/* No suffixes */
@@ -2319,7 +2319,7 @@ static void test_qemu_strtosz_invalid(void)
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
/* No negative values */
@@ -2327,14 +2327,14 @@ static void test_qemu_strtosz_invalid(void)
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
str = "-1";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
}
@@ -2350,65 +2350,65 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz_MiB(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 123 * MiB);
+ g_assert_cmpuint(res, ==, 123 * MiB);
g_assert_true(endptr == str + 3);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
str = "1kiB";
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 1024);
+ g_assert_cmpuint(res, ==, 1024);
g_assert_true(endptr == str + 2);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
str = "0x";
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 0);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str + 1);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
str = "0.NaN";
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 0);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str + 2);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
str = "123-45";
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 123);
+ g_assert_cmpuint(res, ==, 123);
g_assert_true(endptr == str + 3);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
}
static void test_qemu_strtosz_erange(void)
@@ -2422,14 +2422,14 @@ static void test_qemu_strtosz_erange(void)
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -ERANGE);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str + 20);
str = "20E";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -ERANGE);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str + 3);
}
@@ -2445,7 +2445,7 @@ static void test_qemu_strtosz_metric(void)
res = 0xbaadf00d;
err = qemu_strtosz_metric(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 12345000);
+ g_assert_cmpuint(res, ==, 12345000);
g_assert_true(endptr == str + 6);
str = "12.345M";
@@ -2453,7 +2453,7 @@ static void test_qemu_strtosz_metric(void)
res = 0xbaadf00d;
err = qemu_strtosz_metric(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 12345000);
+ g_assert_cmpuint(res, ==, 12345000);
g_assert_true(endptr == str + 7);
}
--
2.40.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 03/11] test-cutils: Test integral qemu_strto* value on failures
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
2023-05-08 20:03 ` [PATCH 01/11] test-cutils: Avoid g_assert in unit tests Eric Blake
2023-05-08 20:03 ` [PATCH 02/11] test-cutils: Use g_assert_cmpuint where appropriate Eric Blake
@ 2023-05-08 20:03 ` Eric Blake
2023-05-08 20:03 ` [PATCH 04/11] test-cutils: Add coverage of qemu_strtod Eric Blake
` (8 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-08 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: hreitz
We are inconsistent on the contents of *value after a strto* parse
failure. I found the following behaviors:
- parse_uint() and parse_uint_full(), which document that *value is
slammed to 0 on all EINVAL failures and 0 or UINT_MAX on ERANGE
failures, and has unit tests for that (note that parse_uint requires
non-NULL endptr, and does not fail with EINVAL for trailing junk)
- qemu_strtosz(), which leaves *value untouched on all failures (both
EINVAL and ERANGE), and has unit tests but not documentation for
that
- qemu_strtoi() and other integral friends, which document *value on
ERANGE failures but is unspecified on EINVAL (other than implicitly
by comparison to libc strto*); there, *value is untouched for NULL
string, slammed to 0 on no conversion, and left at the prefix value
on NULL endptr; unit tests do not consistently check the value
- qemu_strtod(), which documents *value on ERANGE failures but is
unspecified on EINVAL; there, *value is untouched for NULL string,
slammed to 0.0 for no conversion, and left at the prefix value on
NULL endptr; there are no unit tests (other than indirectly through
qemu_strtosz)
- qemu_strtod_finite(), which documents *value on ERANGE failures but
is unspecified on EINVAL; there, *value is left at the prefix for
'inf' or 'nan' and untouched in all other cases; there are no unit
tests (other than indirectly through qemu_strtosz)
Upcoming patches will change behaviors for consistency, but it's best
to first have more unit test coverage to see the impact of those
changes.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/unit/test-cutils.c | 58 +++++++++++++++++++++++++++++++++++-----
1 file changed, 51 insertions(+), 7 deletions(-)
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 38bd3990207..1eeaf21ae22 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -248,6 +248,7 @@ static void test_qemu_strtoi_null(void)
err = qemu_strtoi(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 999);
g_assert_null(endptr);
}
@@ -262,6 +263,7 @@ static void test_qemu_strtoi_empty(void)
err = qemu_strtoi(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -276,6 +278,7 @@ static void test_qemu_strtoi_whitespace(void)
err = qemu_strtoi(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -290,6 +293,7 @@ static void test_qemu_strtoi_invalid(void)
err = qemu_strtoi(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -473,6 +477,7 @@ static void test_qemu_strtoi_full_null(void)
err = qemu_strtoi(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 999);
g_assert_null(endptr);
}
@@ -485,6 +490,7 @@ static void test_qemu_strtoi_full_empty(void)
err = qemu_strtoi(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0);
}
static void test_qemu_strtoi_full_negative(void)
@@ -502,18 +508,19 @@ static void test_qemu_strtoi_full_negative(void)
static void test_qemu_strtoi_full_trailing(void)
{
const char *str = "123xxx";
- int res;
+ int res = 999;
int err;
err = qemu_strtoi(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 123);
}
static void test_qemu_strtoi_full_max(void)
{
char *str = g_strdup_printf("%d", INT_MAX);
- int res;
+ int res = 999;
int err;
err = qemu_strtoi(str, NULL, 0, &res);
@@ -548,6 +555,7 @@ static void test_qemu_strtoui_null(void)
err = qemu_strtoui(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 999);
g_assert_null(endptr);
}
@@ -562,6 +570,7 @@ static void test_qemu_strtoui_empty(void)
err = qemu_strtoui(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -576,6 +585,7 @@ static void test_qemu_strtoui_whitespace(void)
err = qemu_strtoui(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -590,6 +600,7 @@ static void test_qemu_strtoui_invalid(void)
err = qemu_strtoui(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -771,6 +782,7 @@ static void test_qemu_strtoui_full_null(void)
err = qemu_strtoui(NULL, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 999);
}
static void test_qemu_strtoui_full_empty(void)
@@ -782,7 +794,9 @@ static void test_qemu_strtoui_full_empty(void)
err = qemu_strtoui(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
}
+
static void test_qemu_strtoui_full_negative(void)
{
const char *str = " \t -321";
@@ -797,12 +811,13 @@ static void test_qemu_strtoui_full_negative(void)
static void test_qemu_strtoui_full_trailing(void)
{
const char *str = "123xxx";
- unsigned int res;
+ unsigned int res = 999;
int err;
err = qemu_strtoui(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 123);
}
static void test_qemu_strtoui_full_max(void)
@@ -843,6 +858,7 @@ static void test_qemu_strtol_null(void)
err = qemu_strtol(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 999);
g_assert_null(endptr);
}
@@ -857,6 +873,7 @@ static void test_qemu_strtol_empty(void)
err = qemu_strtol(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -871,6 +888,7 @@ static void test_qemu_strtol_whitespace(void)
err = qemu_strtol(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -885,6 +903,7 @@ static void test_qemu_strtol_invalid(void)
err = qemu_strtol(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -1066,6 +1085,7 @@ static void test_qemu_strtol_full_null(void)
err = qemu_strtol(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 999);
g_assert_null(endptr);
}
@@ -1078,6 +1098,7 @@ static void test_qemu_strtol_full_empty(void)
err = qemu_strtol(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0);
}
static void test_qemu_strtol_full_negative(void)
@@ -1095,18 +1116,19 @@ static void test_qemu_strtol_full_negative(void)
static void test_qemu_strtol_full_trailing(void)
{
const char *str = "123xxx";
- long res;
+ long res = 999;
int err;
err = qemu_strtol(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 123);
}
static void test_qemu_strtol_full_max(void)
{
char *str = g_strdup_printf("%ld", LONG_MAX);
- long res;
+ long res = 999;
int err;
err = qemu_strtol(str, NULL, 0, &res);
@@ -1141,6 +1163,7 @@ static void test_qemu_strtoul_null(void)
err = qemu_strtoul(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 999);
g_assert_null(endptr);
}
@@ -1155,6 +1178,7 @@ static void test_qemu_strtoul_empty(void)
err = qemu_strtoul(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -1169,6 +1193,7 @@ static void test_qemu_strtoul_whitespace(void)
err = qemu_strtoul(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -1183,6 +1208,7 @@ static void test_qemu_strtoul_invalid(void)
err = qemu_strtoul(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -1362,6 +1388,7 @@ static void test_qemu_strtoul_full_null(void)
err = qemu_strtoul(NULL, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 999);
}
static void test_qemu_strtoul_full_empty(void)
@@ -1373,7 +1400,9 @@ static void test_qemu_strtoul_full_empty(void)
err = qemu_strtoul(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
}
+
static void test_qemu_strtoul_full_negative(void)
{
const char *str = " \t -321";
@@ -1388,12 +1417,13 @@ static void test_qemu_strtoul_full_negative(void)
static void test_qemu_strtoul_full_trailing(void)
{
const char *str = "123xxx";
- unsigned long res;
+ unsigned long res = 999;
int err;
err = qemu_strtoul(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 123);
}
static void test_qemu_strtoul_full_max(void)
@@ -1434,6 +1464,7 @@ static void test_qemu_strtoi64_null(void)
err = qemu_strtoi64(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 999);
g_assert_null(endptr);
}
@@ -1448,6 +1479,7 @@ static void test_qemu_strtoi64_empty(void)
err = qemu_strtoi64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -1462,6 +1494,7 @@ static void test_qemu_strtoi64_whitespace(void)
err = qemu_strtoi64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -1476,6 +1509,7 @@ static void test_qemu_strtoi64_invalid(void)
err = qemu_strtoi64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -1655,6 +1689,7 @@ static void test_qemu_strtoi64_full_null(void)
err = qemu_strtoi64(NULL, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 999);
}
static void test_qemu_strtoi64_full_empty(void)
@@ -1666,6 +1701,7 @@ static void test_qemu_strtoi64_full_empty(void)
err = qemu_strtoi64(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0);
}
static void test_qemu_strtoi64_full_negative(void)
@@ -1689,13 +1725,14 @@ static void test_qemu_strtoi64_full_trailing(void)
err = qemu_strtoi64(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 123);
}
static void test_qemu_strtoi64_full_max(void)
{
char *str = g_strdup_printf("%lld", LLONG_MAX);
- int64_t res;
+ int64_t res = 999;
int err;
err = qemu_strtoi64(str, NULL, 0, &res);
@@ -1730,6 +1767,7 @@ static void test_qemu_strtou64_null(void)
err = qemu_strtou64(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 999);
g_assert_null(endptr);
}
@@ -1744,6 +1782,7 @@ static void test_qemu_strtou64_empty(void)
err = qemu_strtou64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -1758,6 +1797,7 @@ static void test_qemu_strtou64_whitespace(void)
err = qemu_strtou64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -1772,6 +1812,7 @@ static void test_qemu_strtou64_invalid(void)
err = qemu_strtou64(str, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
}
@@ -1951,6 +1992,7 @@ static void test_qemu_strtou64_full_null(void)
err = qemu_strtou64(NULL, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 999);
}
static void test_qemu_strtou64_full_empty(void)
@@ -1962,6 +2004,7 @@ static void test_qemu_strtou64_full_empty(void)
err = qemu_strtou64(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
}
static void test_qemu_strtou64_full_negative(void)
@@ -1985,6 +2028,7 @@ static void test_qemu_strtou64_full_trailing(void)
err = qemu_strtou64(str, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 18446744073709551614ULL);
}
static void test_qemu_strtou64_full_max(void)
--
2.40.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 04/11] test-cutils: Add coverage of qemu_strtod
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
` (2 preceding siblings ...)
2023-05-08 20:03 ` [PATCH 03/11] test-cutils: Test integral qemu_strto* value on failures Eric Blake
@ 2023-05-08 20:03 ` Eric Blake
2023-05-08 20:03 ` [PATCH 05/11] test-cutils: Prepare for upcoming semantic change in qemu_strtosz Eric Blake
` (7 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-08 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: hreitz
Plenty more corner cases of strtod proper, but this covers the bulk of
what our wrappers do. In particular, it demonstrates the difference on
when *value is left uninitialized, which an upcoming patch will
normalize.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/unit/test-cutils.c | 435 +++++++++++++++++++++++++++++++++++++++
1 file changed, 435 insertions(+)
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 1eeaf21ae22..4c096c6fc70 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -25,6 +25,8 @@
* THE SOFTWARE.
*/
+#include <math.h>
+
#include "qemu/osdep.h"
#include "qemu/cutils.h"
#include "qemu/units.h"
@@ -2044,6 +2046,414 @@ static void test_qemu_strtou64_full_max(void)
g_free(str);
}
+static void test_qemu_strtod_simple(void)
+{
+ const char *str;
+ const char *endptr;
+ int err;
+ double res;
+
+ /* no radix or exponent */
+ str = "1";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpfloat(res, ==, 1.0);
+ g_assert_true(endptr == str + 1);
+
+ /* leading space and sign */
+ str = " -0.0";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpfloat(res, ==, -0.0);
+ g_assert_true(signbit(res));
+ g_assert_true(endptr == str + 5);
+
+ /* fraction only */
+ str = "+.5";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpfloat(res, ==, 0.5);
+ g_assert_true(endptr == str + 3);
+
+ /* exponent */
+ str = "1.e+1";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpfloat(res, ==, 10.0);
+ g_assert_true(endptr == str + 5);
+}
+
+static void test_qemu_strtod_einval(void)
+{
+ const char *str;
+ const char *endptr;
+ int err;
+ double res;
+
+ /* empty */
+ str = "";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_true(endptr == str);
+
+ /* NULL */
+ str = NULL;
+ endptr = "random";
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_null(endptr);
+
+ /* not recognizable */
+ str = " junk";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_true(endptr == str);
+}
+
+static void test_qemu_strtod_erange(void)
+{
+ const char *str;
+ const char *endptr;
+ int err;
+ double res;
+
+ /* overflow */
+ str = "9e999";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -ERANGE);
+ g_assert_cmpfloat(res, ==, HUGE_VAL);
+ g_assert_true(endptr == str + 5);
+
+ str = "-9e+999";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -ERANGE);
+ g_assert_cmpfloat(res, ==, -HUGE_VAL);
+ g_assert_true(endptr == str + 7);
+
+ /* underflow */
+ str = "-9e-999";
+ endptr = NULL;
+ 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_true(signbit(res));
+ g_assert_true(endptr == str + 7);
+}
+
+static void test_qemu_strtod_nonfinite(void)
+{
+ const char *str;
+ const char *endptr;
+ int err;
+ double res;
+
+ /* infinity */
+ str = "inf";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_true(isinf(res));
+ g_assert_false(signbit(res));
+ g_assert_true(endptr == str + 3);
+
+ str = "-infinity";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_true(isinf(res));
+ g_assert_true(signbit(res));
+ g_assert_true(endptr == str + 9);
+
+ /* not a number */
+ str = " NaN";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_true(isnan(res));
+ g_assert_true(endptr == str + 4);
+}
+
+static void test_qemu_strtod_trailing(void)
+{
+ const char *str;
+ const char *endptr;
+ int err;
+ double res;
+
+ /* trailing whitespace */
+ str = "1. ";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpfloat(res, ==, 1.0);
+ g_assert_true(endptr == str + 2);
+
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 1.0);
+
+ /* trailing e is not an exponent */
+ str = ".5e";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpfloat(res, ==, 0.5);
+ g_assert_true(endptr == str + 2);
+
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 0.5);
+
+ /* trailing ( not part of long NaN */
+ str = "nan(";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_true(isnan(res));
+ g_assert_true(endptr == str + 3);
+
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_true(isnan(res));
+}
+
+static void test_qemu_strtod_finite_simple(void)
+{
+ const char *str;
+ const char *endptr;
+ int err;
+ double res;
+
+ /* no radix or exponent */
+ str = "1";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpfloat(res, ==, 1.0);
+ g_assert_true(endptr == str + 1);
+
+ /* leading space and sign */
+ str = " -0.0";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpfloat(res, ==, -0.0);
+ g_assert_true(signbit(res));
+ g_assert_true(endptr == str + 5);
+
+ /* fraction only */
+ str = "+.5";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpfloat(res, ==, 0.5);
+ g_assert_true(endptr == str + 3);
+
+ /* exponent */
+ str = "1.e+1";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpfloat(res, ==, 10.0);
+ g_assert_true(endptr == str + 5);
+}
+
+static void test_qemu_strtod_finite_einval(void)
+{
+ const char *str;
+ const char *endptr;
+ int err;
+ double res;
+
+ /* empty */
+ str = "";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_true(endptr == str);
+
+ /* NULL */
+ str = NULL;
+ endptr = "random";
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_null(endptr);
+
+ /* not recognizable */
+ str = " junk";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_true(endptr == str);
+}
+
+static void test_qemu_strtod_finite_erange(void)
+{
+ const char *str;
+ const char *endptr;
+ int err;
+ double res;
+
+ /* overflow */
+ str = "9e999";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -ERANGE);
+ g_assert_cmpfloat(res, ==, HUGE_VAL);
+ g_assert_true(endptr == str + 5);
+
+ str = "-9e+999";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -ERANGE);
+ g_assert_cmpfloat(res, ==, -HUGE_VAL);
+ g_assert_true(endptr == str + 7);
+
+ /* underflow */
+ str = "-9e-999";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -ERANGE);
+ g_assert_cmpfloat(res, >=, -DBL_MIN);
+ g_assert_cmpfloat(res, <=, -0.0);
+ g_assert_true(signbit(res));
+ g_assert_true(endptr == str + 7);
+}
+
+static void test_qemu_strtod_finite_nonfinite(void)
+{
+ const char *str;
+ const char *endptr;
+ int err;
+ double res;
+
+ /* infinity */
+ str = "inf";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_true(endptr == str);
+
+ str = "-infinity";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_true(endptr == str);
+
+ /* not a number */
+ str = " NaN";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_true(endptr == str);
+}
+
+static void test_qemu_strtod_finite_trailing(void)
+{
+ const char *str;
+ const char *endptr;
+ int err;
+ double res;
+
+ /* trailing whitespace */
+ str = "1. ";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpfloat(res, ==, 1.0);
+ g_assert_true(endptr == str + 2);
+
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 999.0);
+
+ /* trailing e is not an exponent */
+ str = ".5e";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpfloat(res, ==, 0.5);
+ g_assert_true(endptr == str + 2);
+
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 999.0);
+
+ /* trailing ( not part of long NaN */
+ str = "nan(";
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_true(endptr == str);
+
+ endptr = NULL;
+ res = 999;
+ err = qemu_strtod_finite(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 999.0);
+}
+
static void test_qemu_strtosz_simple(void)
{
const char *str;
@@ -2833,6 +3243,31 @@ int main(int argc, char **argv)
g_test_add_func("/cutils/qemu_strtou64_full/max",
test_qemu_strtou64_full_max);
+ /* qemu_strtod() tests */
+ g_test_add_func("/cutils/qemu_strtod/simple",
+ test_qemu_strtod_simple);
+ g_test_add_func("/cutils/qemu_strtod/einval",
+ test_qemu_strtod_einval);
+ g_test_add_func("/cutils/qemu_strtod/erange",
+ test_qemu_strtod_erange);
+ g_test_add_func("/cutils/qemu_strtod/nonfinite",
+ test_qemu_strtod_nonfinite);
+ g_test_add_func("/cutils/qemu_strtod/trailing",
+ test_qemu_strtod_trailing);
+
+ /* qemu_strtod_finite() tests */
+ g_test_add_func("/cutils/qemu_strtod_finite/simple",
+ test_qemu_strtod_finite_simple);
+ g_test_add_func("/cutils/qemu_strtod_finite/einval",
+ test_qemu_strtod_finite_einval);
+ g_test_add_func("/cutils/qemu_strtod_finite/erange",
+ test_qemu_strtod_finite_erange);
+ g_test_add_func("/cutils/qemu_strtod_finite/nonfinite",
+ test_qemu_strtod_finite_nonfinite);
+ g_test_add_func("/cutils/qemu_strtod_finite/trailing",
+ test_qemu_strtod_finite_trailing);
+
+ /* qemu_strtosz() tests */
g_test_add_func("/cutils/strtosz/simple",
test_qemu_strtosz_simple);
g_test_add_func("/cutils/strtosz/hex",
--
2.40.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 05/11] test-cutils: Prepare for upcoming semantic change in qemu_strtosz
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
` (3 preceding siblings ...)
2023-05-08 20:03 ` [PATCH 04/11] test-cutils: Add coverage of qemu_strtod Eric Blake
@ 2023-05-08 20:03 ` Eric Blake
2023-05-08 20:03 ` [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz Eric Blake
` (6 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-08 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: hreitz
A quick search for 'qemu_strtosz' in the code base shows that outside
of the testsuite, the ONLY place that passes a non-NULL pointer to
@endptr of any variant of a size parser is in hmp.c (the 'o' parser of
monitor_parse_arguments), and that particular caller warns of
"extraneous characters at the end of line" unless the trailing bytes
are purely whitespace. Thus, it makes no semantic difference at the
high level whether we parse "1.5e1k" as "1" + ".5e1" + "k" (an attempt
to use scientific notation in strtod with a scaling suffix of 'k' with
no trailing junk, but which qemu_strtosz says should fail with
EINVAL), or as "1.5e" + "1k" (a valid size with scaling suffix of 'e'
for exabytes, followed by two junk bytes) - either way, any user
passing such a string will get an error message about a parse failure.
However, an upcoming patch to qemu_strtosz will fix other corner case
bugs in handling the fractional portion of a size, and in doing so, it
is easier to declare that qemu_strtosz() itself stops parsing at the
first 'e' rather than blindly consuming whatever strtod() will
recognize. Once that is fixed, the difference will be visible at the
low level (getting a valid parse with trailing garbage when @endptr is
non-NULL, while continuing to get -EINVAL when @endptr is NULL); this
is easier to demonstrate by moving the affected strings from
test_qemu_strtosz_invalid() (which declares them as always -EINVAL) to
test_qemu_strtosz_trailing() (where @endptr affects behavior, for now
with FIXME comments).
Note that a similar argument could be made for having "0x1.5" or
"0x1M" parse as 0x1 with ".5" or "M" as trailing junk, instead of
blindly treating it as -EINVAL; however, as these cases do not suffer
from the same problems as floating point, they are not worth changing
at this time.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/unit/test-cutils.c | 42 ++++++++++++++++++++++++++--------------
1 file changed, 27 insertions(+), 15 deletions(-)
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 4c096c6fc70..afae2ee5331 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -2745,21 +2745,6 @@ static void test_qemu_strtosz_invalid(void)
g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
- /* No floating point exponents */
- str = "1.5e1k";
- endptr = NULL;
- err = qemu_strtosz(str, &endptr, &res);
- g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
- g_assert_true(endptr == str);
-
- str = "1.5E+0k";
- endptr = NULL;
- err = qemu_strtosz(str, &endptr, &res);
- g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
- g_assert_true(endptr == str);
-
/* No hex fractions */
str = "0x1.8k";
endptr = NULL;
@@ -2863,6 +2848,33 @@ static void test_qemu_strtosz_trailing(void)
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
+
+ /* FIXME should stop parse after 'e'. No floating point exponents */
+ str = "1.5e1k";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL /* FIXME 0 */);
+ g_assert_cmphex(res, ==, 0xbaadf00d /* FIXME EiB * 1.5 */);
+ g_assert_true(endptr == str /* FIXME + 4 */);
+
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpint(res, ==, 0xbaadf00d);
+
+ str = "1.5E+0k";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL /* FIXME 0 */);
+ g_assert_cmphex(res, ==, 0xbaadf00d /* FIXME EiB * 1.5 */);
+ g_assert_true(endptr == str /* FIXME + 4 */);
+
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
}
static void test_qemu_strtosz_erange(void)
--
2.40.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
` (4 preceding siblings ...)
2023-05-08 20:03 ` [PATCH 05/11] test-cutils: Prepare for upcoming semantic change in qemu_strtosz Eric Blake
@ 2023-05-08 20:03 ` Eric Blake
2023-05-09 12:31 ` Hanna Czenczek
` (2 more replies)
2023-05-08 20:03 ` [PATCH 07/11] numa: Check for qemu_strtosz_MiB error Eric Blake
` (5 subsequent siblings)
11 siblings, 3 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-08 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: hreitz
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
@@ -2478,14 +2478,14 @@ static void test_qemu_strtosz_simple(void)
g_assert_cmpuint(res, ==, 8);
g_assert_true(endptr == str + 2);
- /* Leading space is ignored */
- str = " 12345";
+ /* Leading space and + are ignored */
+ str = " +12345";
endptr = str;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 12345);
- g_assert_true(endptr == str + 6);
+ g_assert_true(endptr == str + 7);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
@@ -2564,13 +2564,13 @@ static void test_qemu_strtosz_hex(void)
g_assert_cmpuint(res, ==, 171);
g_assert_true(endptr == str + 4);
- str = "0xae";
+ str = " +0xae";
endptr = str;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 174);
- g_assert_true(endptr == str + 4);
+ g_assert_true(endptr == str + 6);
}
static void test_qemu_strtosz_units(void)
@@ -2669,14 +2669,23 @@ static void test_qemu_strtosz_float(void)
g_assert_cmpuint(res, ==, 1);
g_assert_true(endptr == str + 4);
- /* An empty fraction is tolerated */
- str = "1.k";
+ /* An empty fraction tail is tolerated */
+ str = " 1.k";
endptr = str;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 1024);
- g_assert_true(endptr == str + 3);
+ g_assert_true(endptr == str + 4);
+
+ /* FIXME An empty fraction head should be tolerated */
+ str = " .5k";
+ endptr = str;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL /* FIXME 0 */);
+ g_assert_cmpuint(res, ==, 0xbaadf00d /* FIXME 512 */);
+ g_assert_true(endptr == str /* FIXME + 4 */);
/* For convenience, we permit values that are not byte-exact */
str = "12.345M";
@@ -2686,6 +2695,32 @@ static void test_qemu_strtosz_float(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, (uint64_t) (12.345 * MiB + 0.5));
g_assert_true(endptr == str + 7);
+
+ /* FIXME Fraction tail should round correctly */
+ str = "1.9999999999999999999999999999999999999999999999999999k";
+ endptr = str;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpint(res, ==, 1024 /* FIXME 2048 */);
+ g_assert_true(endptr == str + 55);
+
+ /* FIXME ERANGE underflow in the fraction tail should not matter for 'k' */
+ str = "1."
+ "00000000000000000000000000000000000000000000000000"
+ "00000000000000000000000000000000000000000000000000"
+ "00000000000000000000000000000000000000000000000000"
+ "00000000000000000000000000000000000000000000000000"
+ "00000000000000000000000000000000000000000000000000"
+ "00000000000000000000000000000000000000000000000000"
+ "00000000000000000000000000000000000000000000000000"
+ "1k";
+ endptr = str;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpuint(res, ==, 1 /* FIXME 1024 */);
+ g_assert_true(endptr == str + 354);
}
static void test_qemu_strtosz_invalid(void)
@@ -2693,10 +2728,20 @@ static void test_qemu_strtosz_invalid(void)
const char *str;
const char *endptr;
int err;
- uint64_t res = 0xbaadf00d;
+ uint64_t res;
+
+ /* Must parse at least one digit */
+ str = NULL;
+ endptr = "somewhere";
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_null(endptr);
str = "";
endptr = NULL;
+ res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
@@ -2704,13 +2749,30 @@ static void test_qemu_strtosz_invalid(void)
str = " \t ";
endptr = NULL;
+ res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
+ str = ".";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert(endptr == str);
+
+ str = " .";
+ endptr = NULL;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert(endptr == str);
+
str = "crap";
endptr = NULL;
+ res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
@@ -2718,6 +2780,7 @@ static void test_qemu_strtosz_invalid(void)
str = "inf";
endptr = NULL;
+ res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
@@ -2725,6 +2788,7 @@ static void test_qemu_strtosz_invalid(void)
str = "NaN";
endptr = NULL;
+ res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
@@ -2733,6 +2797,7 @@ static void test_qemu_strtosz_invalid(void)
/* Fractional values require scale larger than bytes */
str = "1.1B";
endptr = NULL;
+ res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
@@ -2740,14 +2805,42 @@ static void test_qemu_strtosz_invalid(void)
str = "1.1";
endptr = NULL;
+ res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
+ /* FIXME Fraction tail can cause ERANGE overflow */
+ str = "15.9999999999999999999999999999999999999999999999999999E";
+ endptr = str;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0 /* FIXME -ERANGE */);
+ g_assert_cmpuint(res, ==, 15ULL * EiB /* FIXME 0xbaadf00d */);
+ g_assert_true(endptr == str + 56 /* FIXME str */);
+
+ /* FIXME ERANGE underflow in the fraction tail should matter for 'B' */
+ str = "1."
+ "00000000000000000000000000000000000000000000000000"
+ "00000000000000000000000000000000000000000000000000"
+ "00000000000000000000000000000000000000000000000000"
+ "00000000000000000000000000000000000000000000000000"
+ "00000000000000000000000000000000000000000000000000"
+ "00000000000000000000000000000000000000000000000000"
+ "00000000000000000000000000000000000000000000000000"
+ "1B";
+ endptr = str;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0 /* FIXME -EINVAL */);
+ g_assert_cmpuint(res, ==, 1 /* FIXME 0xbaadf00d */);
+ g_assert_true(endptr == str + 354 /* FIXME str */);
+
/* No hex fractions */
str = "0x1.8k";
endptr = NULL;
+ res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
@@ -2756,14 +2849,24 @@ static void test_qemu_strtosz_invalid(void)
/* No suffixes */
str = "0x18M";
endptr = NULL;
+ res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
+ str = "0x1p1";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert(endptr == str);
+
/* No negative values */
- str = "-0";
+ str = " -0";
endptr = NULL;
+ res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
@@ -2771,10 +2874,28 @@ static void test_qemu_strtosz_invalid(void)
str = "-1";
endptr = NULL;
+ res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
g_assert_true(endptr == str);
+
+ str = "-.0k";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_true(endptr == str);
+
+ /* Too many decimals */
+ str = "1.1.k";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert(endptr == str);
}
static void test_qemu_strtosz_trailing(void)
@@ -2784,6 +2905,21 @@ static void test_qemu_strtosz_trailing(void)
int err;
uint64_t res;
+ /* Trailing whitespace */
+ str = "1k ";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpuint(res, ==, 1024);
+ g_assert(endptr == str + 2);
+
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
+
+ /* Unknown suffix */
str = "123xxx";
endptr = NULL;
res = 0xbaadf00d;
@@ -2797,6 +2933,7 @@ static void test_qemu_strtosz_trailing(void)
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
+ /* Junk after one-byte suffix */
str = "1kiB";
endptr = NULL;
res = 0xbaadf00d;
@@ -2810,6 +2947,7 @@ static void test_qemu_strtosz_trailing(void)
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
+ /* Incomplete hex is an unknown suffix */
str = "0x";
endptr = NULL;
res = 0xbaadf00d;
@@ -2823,6 +2961,7 @@ static void test_qemu_strtosz_trailing(void)
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
+ /* Junk after decimal */
str = "0.NaN";
endptr = NULL;
res = 0xbaadf00d;
@@ -2836,6 +2975,35 @@ static void test_qemu_strtosz_trailing(void)
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
+ /* No support for binary literals; 'b' is valid suffix */
+ str = "0b1000";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpuint(res, ==, 0);
+ g_assert(endptr == str + 2);
+
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
+
+ /* Hex literals use only one leading zero */
+ str = "00x1";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpuint(res, ==, 0);
+ g_assert(endptr == str + 2);
+
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
+
+ /* Although negatives are invalid, '-' may be in trailing junk */
str = "123-45";
endptr = NULL;
res = 0xbaadf00d;
@@ -2849,6 +3017,19 @@ static void test_qemu_strtosz_trailing(void)
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmphex(res, ==, 0xbaadf00d);
+ str = " 123 - 45";
+ endptr = NULL;
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpuint(res, ==, 123);
+ g_assert(endptr == str + 4);
+
+ res = 0xbaadf00d;
+ err = qemu_strtosz(str, NULL, &res);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
+
/* FIXME should stop parse after 'e'. No floating point exponents */
str = "1.5e1k";
endptr = NULL;
@@ -2861,7 +3042,7 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 0xbaadf00d);
+ g_assert_cmphex(res, ==, 0xbaadf00d);
str = "1.5E+0k";
endptr = NULL;
@@ -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);
}
static void test_qemu_strtosz_erange(void)
@@ -2921,6 +3116,15 @@ static void test_qemu_strtosz_metric(void)
g_assert_cmpint(err, ==, 0);
g_assert_cmpuint(res, ==, 12345000);
g_assert_true(endptr == str + 7);
+
+ /* Fraction is affected by floating-point rounding */
+ str = "18.446744073709550591E"; /* resembles 0xfffffffffffffbff */
+ endptr = str;
+ res = 0xbaadf00d;
+ err = qemu_strtosz_metric(str, &endptr, &res);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpuint(res, ==, 0xfffffffffffffc0c);
+ g_assert_true(endptr == str + 22);
}
static void test_freq_to_str(void)
--
2.40.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz
2023-05-08 20:03 ` [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz Eric Blake
@ 2023-05-09 12:31 ` Hanna Czenczek
2023-05-09 12:42 ` Hanna Czenczek
2023-05-09 16:06 ` Eric Blake
2023-05-09 15:15 ` Hanna Czenczek
2023-05-09 16:10 ` Eric Blake
2 siblings, 2 replies; 25+ messages in thread
From: Hanna Czenczek @ 2023-05-09 12:31 UTC (permalink / raw)
To: Eric Blake, qemu-devel
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);
> }
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz
2023-05-09 12:31 ` Hanna Czenczek
@ 2023-05-09 12:42 ` Hanna Czenczek
2023-05-09 16:06 ` Eric Blake
1 sibling, 0 replies; 25+ messages in thread
From: Hanna Czenczek @ 2023-05-09 12:42 UTC (permalink / raw)
To: Eric Blake, qemu-devel
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 <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 */);
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz
2023-05-09 12:31 ` Hanna Czenczek
2023-05-09 12:42 ` Hanna Czenczek
@ 2023-05-09 16:06 ` Eric Blake
1 sibling, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-09 16:06 UTC (permalink / raw)
To: Hanna Czenczek; +Cc: qemu-devel
On Tue, May 09, 2023 at 02:31:08PM +0200, 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 <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?
Pre-series: "1.5e+1k" is parsed as "1" ".5e+1" "k", no slop
Post-series: "1.5e+1k" is parsed as "1" ".5" "e", slop "+1k"
If you call qemu_strtosz(,NULL,), both versions still give EINVAL
error (pre-patch: we detected an exponent in the middle portion of the
parse, which is not allowed; post-series: we detect slop, which is not
allowed). If you call qemu_strtosz(,&endptr), the pre-patch version
fails with EINVAL but the new code succeeds. But of all the callers,
the only one that passed non-NULL endptr did it's own check that *slop
is only whitespace ("+1k" fails that check), so the end result is that
at the high level, we reject "1.5e+1k" from the user, but the
rejection is now at a different point in the call stack. The reason I
made the change is that it was less code to guarantee that the
internal qemu_strtod_finite() is passed a string that cannot possibly
contain an exponent, than to blindly call qemu_strtod_finite() on an
arbitrary suffix and then check after the fact on whether an exponent
was actually consumed as part of that parse.
For "0x1p1", the parse is "0x1" plus slop "p1" (unchanged by the
series). This is rejected with EINVAL (the moment you have hex, we
insist on no suffix or slop, regardless of endptr being NULL or not).
I could have changed it similar to how I changed "1.5e+1k" to allow
the parse at the qemu_strtosz layer and then detect the slop at the
caller, but that was more lines of code and I didn't feel like it was
worth it.
Either way, the unit tests accurately cover the difference in parse
behavior, and I tried to make the documentation (by patch 11/11) be
consistent as well. But since this is why we have a review process,
I'm open to feedback if you think I need to tweak which parse styles
we should be favoring.
>
> (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.)
"1.1k" is a valid (if unusual) size. This particular test addition
did not happen to improve coverage for my final code, but given that
qemu_strtod_finite(".1.k") would stop parsing after the ".1" and still
be pointing at '.', and our read-out-of-bounds was caused by assuming
that qemu_strtod_finite() failures leave *endptr == '.' (which was
invalidated for ".9e999" failing with ERANGE), it didn't hurt to add
the coverage.
>
> I don’t think it matters to users, really, but I still wonder.
If any of our callers had actually done something like: "size %llx
followed by junk %s" with the parsed value and the junk string, then
it would matter. But since all of the callers either passed in NULL
endptr (and got EINVAL before even knowing how much of the string was
parsed) or insisted that the junk be whitespace only, and did not
print details about the parsed value before reporting such errors,
you're right that I don't see this as mattering to end users. But it
took me quite a bit of audit time to convince myself of that.
> > @@ -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.
Huh - wonder what build settings you are using that I wasn't for
seeing that warning - but it makes total sense that 'str + 9' would
trigger a build failure if we don't pad str with enough '\0' to keep
things in bounds.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz
2023-05-08 20:03 ` [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz Eric Blake
2023-05-09 12:31 ` Hanna Czenczek
@ 2023-05-09 15:15 ` Hanna Czenczek
2023-05-09 15:50 ` Eric Blake
2023-05-09 16:10 ` Eric Blake
2 siblings, 1 reply; 25+ messages in thread
From: Hanna Czenczek @ 2023-05-09 15:15 UTC (permalink / raw)
To: Eric Blake, qemu-devel
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz
2023-05-09 15:15 ` Hanna Czenczek
@ 2023-05-09 15:50 ` Eric Blake
0 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-09 15:50 UTC (permalink / raw)
To: Hanna Czenczek; +Cc: qemu-devel
On Tue, May 09, 2023 at 05:15:04PM +0200, 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 <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.
Wow, yeah, that's a mess. The very test proving that we have a
read-out-of-bounds bug is super-sensitive to what is at that location.
Your hack of passing in extra \0 is awesome; I'll fold that in whether
we need a v2 for other reasons, or in-place if we can take the rest of
this series as-is. It all goes away at the end of the series,
anyways, once the out-of-bounds read is fixed.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz
2023-05-08 20:03 ` [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz Eric Blake
2023-05-09 12:31 ` Hanna Czenczek
2023-05-09 15:15 ` Hanna Czenczek
@ 2023-05-09 16:10 ` Eric Blake
2 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-09 16:10 UTC (permalink / raw)
To: qemu-devel; +Cc: hreitz
On Mon, May 08, 2023 at 03:03:38PM -0500, 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(-)
>
> @@ -2704,13 +2749,30 @@ static void test_qemu_strtosz_invalid(void)
>
> str = " \t ";
> endptr = NULL;
> + res = 0xbaadf00d;
> err = qemu_strtosz(str, &endptr, &res);
> g_assert_cmpint(err, ==, -EINVAL);
> g_assert_cmphex(res, ==, 0xbaadf00d);
> g_assert_true(endptr == str);
>
> + str = ".";
> + endptr = NULL;
> + res = 0xbaadf00d;
> + err = qemu_strtosz(str, &endptr, &res);
> + g_assert_cmpint(err, ==, -EINVAL);
> + g_assert_cmphex(res, ==, 0xbaadf00d);
> + g_assert(endptr == str);
Rebase botch. I should be using g_assert_true() here in line with
earlier in the series. I think I cleaned it up later in the series,
but shouldn't be churning on it that badly. Looks like I get to send
a v2 to fix this and other things; I'll wait another day for other
reviews first. (That's what I get for rearranging patches after the
fact for a nicer presentation order...)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 07/11] numa: Check for qemu_strtosz_MiB error
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
` (5 preceding siblings ...)
2023-05-08 20:03 ` [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz Eric Blake
@ 2023-05-08 20:03 ` Eric Blake
2023-05-08 21:15 ` Eric Blake
2023-05-08 20:03 ` [PATCH 08/11] cutils: Set value in all qemu_strtosz* error paths Eric Blake
` (4 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Eric Blake @ 2023-05-08 20:03 UTC (permalink / raw)
To: qemu-devel
Cc: hreitz, Eduardo Habkost, Marcel Apfelbaum,
Philippe Mathieu-Daudé, Yanan Wang
As shown in the previous commit, qemu_strtosz_MiB sometimes leaves the
result value untoutched (we have to audit further to learn that in
that case, the QAPI generator says that visit_type_NumaOptions() will
have zero-initialized it), and sometimes leaves it with the value of a
partial parse before -EINVAL occurs because of trailing garbage.
Rather than blindly treating any string the user may throw at us as
valid, we should check for parse failures.
Fiuxes: cc001888 ("numa: fixup parsed NumaNodeOptions earlier", v2.11.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
---
hw/core/numa.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/hw/core/numa.c b/hw/core/numa.c
index d8d36b16d80..f08956ddb0f 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -531,10 +531,17 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
/* Fix up legacy suffix-less format */
if ((object->type == NUMA_OPTIONS_TYPE_NODE) && object->u.node.has_mem) {
const char *mem_str = qemu_opt_get(opts, "mem");
- qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem);
+ int ret = qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem);
+
+ if (ret < 0) {
+ error_setg_errno(&err, -ret, "could not parse memory size '%s'",
+ mem_str);
+ }
}
- set_numa_options(ms, object, &err);
+ if (!err) {
+ set_numa_options(ms, object, &err);
+ }
qapi_free_NumaOptions(object);
if (err) {
--
2.40.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 07/11] numa: Check for qemu_strtosz_MiB error
2023-05-08 20:03 ` [PATCH 07/11] numa: Check for qemu_strtosz_MiB error Eric Blake
@ 2023-05-08 21:15 ` Eric Blake
0 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-08 21:15 UTC (permalink / raw)
To: qemu-devel
Cc: hreitz, Eduardo Habkost, Marcel Apfelbaum,
Philippe Mathieu-Daudé, Yanan Wang
On Mon, May 08, 2023 at 03:03:39PM -0500, Eric Blake wrote:
> As shown in the previous commit, qemu_strtosz_MiB sometimes leaves the
> result value untoutched (we have to audit further to learn that in
untouched
> that case, the QAPI generator says that visit_type_NumaOptions() will
> have zero-initialized it), and sometimes leaves it with the value of a
> partial parse before -EINVAL occurs because of trailing garbage.
> Rather than blindly treating any string the user may throw at us as
> valid, we should check for parse failures.
>
> Fiuxes: cc001888 ("numa: fixup parsed NumaNodeOptions earlier", v2.11.0)
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 08/11] cutils: Set value in all qemu_strtosz* error paths
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
` (6 preceding siblings ...)
2023-05-08 20:03 ` [PATCH 07/11] numa: Check for qemu_strtosz_MiB error Eric Blake
@ 2023-05-08 20:03 ` Eric Blake
2023-05-08 20:03 ` [PATCH 09/11] cutils: Set value in all integral qemu_strto* " Eric Blake
` (3 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-08 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: hreitz
Making callers determine whether or not *value was populated on error
is not nice for usability. Pre-patch, we have unit tests that check
that *result is left unchanged on most EINVAL errors and set to 0 on
many ERANGE errors. This is subtly different from libc strtoumax()
behavior which returns UINT64_MAX on ERANGE errors, as well as
different from our parse_uint() which slams to 0 on EINVAL on the
grounds that we want our functions to be harder to mis-use than
strtoumax().
Let's audit callers:
- hw/core/numa.c:parse_numa() fixed in the previous patch to check for
errors
- migration/migration-hmp-cmds.c:hmp_migrate_set_parameter(),
monitor/hmp.c:monitor_parse_arguments(),
qapi/opts-visitor.c:opts_type_size(),
qapi/qobject-input-visitor.c:qobject_input_type_size_keyval(),
qemu-img.c:cvtnum_full(), qemu-io-cmds.c:cvtnum(),
target/i386/cpu.c:x86_cpu_parse_featurestr(), and
util/qemu-option.c:parse_option_size() appear to reject all failures
(although some with distinct messages for ERANGE as opposed to
EINVAL), so it doesn't matter what is in the value parameter on
error.
- All remaining callers are in the testsuite, where we can tweak our
expectations to match our new desired behavior.
Advancing to the end of the string parsed on overflow (ERANGE), while
still returning 0, makes sense (UINT64_MAX as a size is unlikely to be
useful); likewise, our size parsing code is complex enough that it's
easier to always return 0 when endptr is NULL but trailing garbage was
found, rather than trying to return the value of the prefix actually
parsed (no current caller cared about the value of the prefix).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/unit/test-cutils.c | 72 ++++++++++++++++++++--------------------
util/cutils.c | 17 +++++++---
2 files changed, 48 insertions(+), 41 deletions(-)
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 9fa6fb042e8..9cf00a810e4 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -2684,7 +2684,7 @@ static void test_qemu_strtosz_float(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL /* FIXME 0 */);
- g_assert_cmpuint(res, ==, 0xbaadf00d /* FIXME 512 */);
+ g_assert_cmpuint(res, ==, 0 /* FIXME 512 */);
g_assert_true(endptr == str /* FIXME + 4 */);
/* For convenience, we permit values that are not byte-exact */
@@ -2736,7 +2736,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_null(endptr);
str = "";
@@ -2744,7 +2744,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
str = " \t ";
@@ -2752,7 +2752,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
str = ".";
@@ -2760,14 +2760,14 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert(endptr == str);
str = " .";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert(endptr == str);
str = "crap";
@@ -2775,7 +2775,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
str = "inf";
@@ -2783,7 +2783,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
str = "NaN";
@@ -2791,7 +2791,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
/* Fractional values require scale larger than bytes */
@@ -2800,7 +2800,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
str = "1.1";
@@ -2808,7 +2808,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
/* FIXME Fraction tail can cause ERANGE overflow */
@@ -2817,7 +2817,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0 /* FIXME -ERANGE */);
- g_assert_cmpuint(res, ==, 15ULL * EiB /* FIXME 0xbaadf00d */);
+ g_assert_cmpuint(res, ==, 15ULL * EiB /* FIXME 0 */);
g_assert_true(endptr == str + 56 /* FIXME str */);
/* FIXME ERANGE underflow in the fraction tail should matter for 'B' */
@@ -2834,7 +2834,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0 /* FIXME -EINVAL */);
- g_assert_cmpuint(res, ==, 1 /* FIXME 0xbaadf00d */);
+ g_assert_cmpuint(res, ==, 1 /* FIXME 0 */);
g_assert_true(endptr == str + 354 /* FIXME str */);
/* No hex fractions */
@@ -2843,7 +2843,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
/* No suffixes */
@@ -2852,7 +2852,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
str = "0x1p1";
@@ -2860,7 +2860,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert(endptr == str);
/* No negative values */
@@ -2869,7 +2869,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
str = "-1";
@@ -2877,7 +2877,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
str = "-.0k";
@@ -2885,7 +2885,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
/* Too many decimals */
@@ -2894,7 +2894,7 @@ static void test_qemu_strtosz_invalid(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert(endptr == str);
}
@@ -2917,7 +2917,7 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
/* Unknown suffix */
str = "123xxx";
@@ -2931,7 +2931,7 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
/* Junk after one-byte suffix */
str = "1kiB";
@@ -2945,7 +2945,7 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
/* Incomplete hex is an unknown suffix */
str = "0x";
@@ -2959,7 +2959,7 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
/* Junk after decimal */
str = "0.NaN";
@@ -2973,7 +2973,7 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
/* No support for binary literals; 'b' is valid suffix */
str = "0b1000";
@@ -2987,7 +2987,7 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
/* Hex literals use only one leading zero */
str = "00x1";
@@ -3001,7 +3001,7 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
/* Although negatives are invalid, '-' may be in trailing junk */
str = "123-45";
@@ -3015,7 +3015,7 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
str = " 123 - 45";
endptr = NULL;
@@ -3028,7 +3028,7 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
/* FIXME should stop parse after 'e'. No floating point exponents */
str = "1.5e1k";
@@ -3036,26 +3036,26 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL /* FIXME 0 */);
- g_assert_cmphex(res, ==, 0xbaadf00d /* FIXME EiB * 1.5 */);
+ g_assert_cmpuint(res, ==, 0 /* FIXME EiB * 1.5 */);
g_assert_true(endptr == str /* FIXME + 4 */);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
str = "1.5E+0k";
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL /* FIXME 0 */);
- g_assert_cmphex(res, ==, 0xbaadf00d /* FIXME EiB * 1.5 */);
+ g_assert_cmpuint(res, ==, 0 /* FIXME EiB * 1.5 */);
g_assert_true(endptr == str /* FIXME + 4 */);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
/* FIXME overflow in fraction is buggy */
str = "1.5E999";
@@ -3069,7 +3069,7 @@ static void test_qemu_strtosz_trailing(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
}
static void test_qemu_strtosz_erange(void)
@@ -3083,14 +3083,14 @@ static void test_qemu_strtosz_erange(void)
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -ERANGE);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str + 20);
str = "20E";
endptr = NULL;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, -ERANGE);
- g_assert_cmphex(res, ==, 0xbaadf00d);
+ g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str + 3);
}
diff --git a/util/cutils.c b/util/cutils.c
index 5887e744140..8bacf349383 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -205,13 +205,15 @@ static int64_t suffix_mul(char suffix, int64_t unit)
*
* The end pointer will be returned in *end, if not NULL. If there is
* no fraction, the input can be decimal or hexadecimal; if there is a
- * fraction, then the input must be decimal and there must be a suffix
- * (possibly by @default_suffix) larger than Byte, and the fractional
- * portion may suffer from precision loss or rounding. The input must
- * be positive.
+ * non-zero fraction, then the input must be decimal and there must be
+ * a suffix (possibly by @default_suffix) larger than Byte, and the
+ * fractional portion may suffer from precision loss or rounding. The
+ * input must be positive.
*
* Return -ERANGE on overflow (with *@end advanced), and -EINVAL on
- * other error (with *@end left unchanged).
+ * other error (with *@end at @nptr). Unlike strtoull, *@result is
+ * set to 0 on all errors, as returning UINT64_MAX on overflow is less
+ * likely to be usable as a size.
*/
static int do_strtosz(const char *nptr, const char **end,
const char default_suffix, int64_t unit,
@@ -311,6 +313,11 @@ out:
}
if (retval == 0) {
*result = val;
+ } else {
+ *result = 0;
+ if (end && retval == -EINVAL) {
+ *end = nptr;
+ }
}
return retval;
--
2.40.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 09/11] cutils: Set value in all integral qemu_strto* error paths
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
` (7 preceding siblings ...)
2023-05-08 20:03 ` [PATCH 08/11] cutils: Set value in all qemu_strtosz* error paths Eric Blake
@ 2023-05-08 20:03 ` Eric Blake
2023-05-08 20:03 ` [PATCH 10/11] cutils: Improve qemu_strtod* " Eric Blake
` (2 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-08 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: hreitz
Our goal in writing qemu_strtoi() and friends is to have an interface
harder to abuse than libc's strtol(). Leaving the return value
initialized on some error paths does not lend itself well to this
goal; and our documentation wasn't helpful on the matter.
Note that the previous patch changed all qemu_strtosz() EINVAL error
paths to slam value to 0 rather than stay uninitialized, even when the
EINVAL eror occurs because of trailing junk. But for the remaining
integral qemu_strto*, it's easier to return the parsed value than to
force things back to zero, in part because of how check_strtox_error
works; and doing so creates less churn in the testsuite.
Here, the list of affected callers is much longer ('git grep
"qemu_strto[ui]" *.c **/*.c | grep -v tests/ |wc -l' outputs 87,
although a few of those are the implementation in in cutils.c), so
touching as little as possible is the wisest course of action.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/unit/test-cutils.c | 24 +++++++++++------------
util/cutils.c | 42 +++++++++++++++++++++++++---------------
2 files changed, 38 insertions(+), 28 deletions(-)
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 9cf00a810e4..2cb33e41ae4 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -250,7 +250,7 @@ static void test_qemu_strtoi_null(void)
err = qemu_strtoi(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 999);
+ g_assert_cmpint(res, ==, 0);
g_assert_null(endptr);
}
@@ -479,7 +479,7 @@ static void test_qemu_strtoi_full_null(void)
err = qemu_strtoi(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 999);
+ g_assert_cmpint(res, ==, 0);
g_assert_null(endptr);
}
@@ -557,7 +557,7 @@ static void test_qemu_strtoui_null(void)
err = qemu_strtoui(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpuint(res, ==, 999);
+ g_assert_cmpuint(res, ==, 0);
g_assert_null(endptr);
}
@@ -784,7 +784,7 @@ static void test_qemu_strtoui_full_null(void)
err = qemu_strtoui(NULL, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpuint(res, ==, 999);
+ g_assert_cmpuint(res, ==, 0);
}
static void test_qemu_strtoui_full_empty(void)
@@ -860,7 +860,7 @@ static void test_qemu_strtol_null(void)
err = qemu_strtol(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 999);
+ g_assert_cmpint(res, ==, 0);
g_assert_null(endptr);
}
@@ -1087,7 +1087,7 @@ static void test_qemu_strtol_full_null(void)
err = qemu_strtol(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 999);
+ g_assert_cmpint(res, ==, 0);
g_assert_null(endptr);
}
@@ -1165,7 +1165,7 @@ static void test_qemu_strtoul_null(void)
err = qemu_strtoul(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpuint(res, ==, 999);
+ g_assert_cmpuint(res, ==, 0);
g_assert_null(endptr);
}
@@ -1390,7 +1390,7 @@ static void test_qemu_strtoul_full_null(void)
err = qemu_strtoul(NULL, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpuint(res, ==, 999);
+ g_assert_cmpuint(res, ==, 0);
}
static void test_qemu_strtoul_full_empty(void)
@@ -1466,7 +1466,7 @@ static void test_qemu_strtoi64_null(void)
err = qemu_strtoi64(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 999);
+ g_assert_cmpint(res, ==, 0);
g_assert_null(endptr);
}
@@ -1691,7 +1691,7 @@ static void test_qemu_strtoi64_full_null(void)
err = qemu_strtoi64(NULL, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpint(res, ==, 999);
+ g_assert_cmpint(res, ==, 0);
}
static void test_qemu_strtoi64_full_empty(void)
@@ -1769,7 +1769,7 @@ static void test_qemu_strtou64_null(void)
err = qemu_strtou64(NULL, &endptr, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpuint(res, ==, 999);
+ g_assert_cmpuint(res, ==, 0);
g_assert_null(endptr);
}
@@ -1994,7 +1994,7 @@ static void test_qemu_strtou64_full_null(void)
err = qemu_strtou64(NULL, NULL, 0, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpuint(res, ==, 999);
+ g_assert_cmpuint(res, ==, 0);
}
static void test_qemu_strtou64_full_empty(void)
diff --git a/util/cutils.c b/util/cutils.c
index 8bacf349383..83948926ec9 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -384,12 +384,13 @@ static int check_strtox_error(const char *nptr, char *ep,
*
* @nptr may be null, and no conversion is performed then.
*
- * If no conversion is performed, store @nptr in *@endptr and return
- * -EINVAL.
+ * If no conversion is performed, store @nptr in *@endptr, 0 in
+ * @result, and return -EINVAL.
*
* If @endptr is null, and the string isn't fully converted, return
- * -EINVAL. This is the case when the pointer that would be stored in
- * a non-null @endptr points to a character other than '\0'.
+ * -EINVAL with @result set to the parsed value. This is the case
+ * when the pointer that would be stored in a non-null @endptr points
+ * to a character other than '\0'.
*
* If the conversion overflows @result, store INT_MAX in @result,
* and return -ERANGE.
@@ -407,6 +408,7 @@ int qemu_strtoi(const char *nptr, const char **endptr, int base,
assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
+ *result = 0;
if (endptr) {
*endptr = nptr;
}
@@ -436,12 +438,13 @@ int qemu_strtoi(const char *nptr, const char **endptr, int base,
*
* @nptr may be null, and no conversion is performed then.
*
- * If no conversion is performed, store @nptr in *@endptr and return
- * -EINVAL.
+ * If no conversion is performed, store @nptr in *@endptr, 0 in
+ * @result, and return -EINVAL.
*
* If @endptr is null, and the string isn't fully converted, return
- * -EINVAL. This is the case when the pointer that would be stored in
- * a non-null @endptr points to a character other than '\0'.
+ * -EINVAL with @result set to the parsed value. This is the case
+ * when the pointer that would be stored in a non-null @endptr points
+ * to a character other than '\0'.
*
* If the conversion overflows @result, store UINT_MAX in @result,
* and return -ERANGE.
@@ -460,6 +463,7 @@ int qemu_strtoui(const char *nptr, const char **endptr, int base,
assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
+ *result = 0;
if (endptr) {
*endptr = nptr;
}
@@ -495,12 +499,13 @@ int qemu_strtoui(const char *nptr, const char **endptr, int base,
*
* @nptr may be null, and no conversion is performed then.
*
- * If no conversion is performed, store @nptr in *@endptr and return
- * -EINVAL.
+ * If no conversion is performed, store @nptr in *@endptr, 0 in
+ * @result, and return -EINVAL.
*
* If @endptr is null, and the string isn't fully converted, return
- * -EINVAL. This is the case when the pointer that would be stored in
- * a non-null @endptr points to a character other than '\0'.
+ * -EINVAL with @result set to the parsed value. This is the case
+ * when the pointer that would be stored in a non-null @endptr points
+ * to a character other than '\0'.
*
* If the conversion overflows @result, store LONG_MAX in @result,
* and return -ERANGE.
@@ -517,6 +522,7 @@ int qemu_strtol(const char *nptr, const char **endptr, int base,
assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
+ *result = 0;
if (endptr) {
*endptr = nptr;
}
@@ -537,12 +543,13 @@ int qemu_strtol(const char *nptr, const char **endptr, int base,
*
* @nptr may be null, and no conversion is performed then.
*
- * If no conversion is performed, store @nptr in *@endptr and return
- * -EINVAL.
+ * If no conversion is performed, store @nptr in *@endptr, 0 in
+ * @result, and return -EINVAL.
*
* If @endptr is null, and the string isn't fully converted, return
- * -EINVAL. This is the case when the pointer that would be stored in
- * a non-null @endptr points to a character other than '\0'.
+ * -EINVAL with @result set to the parsed value. This is the case
+ * when the pointer that would be stored in a non-null @endptr points
+ * to a character other than '\0'.
*
* If the conversion overflows @result, store ULONG_MAX in @result,
* and return -ERANGE.
@@ -560,6 +567,7 @@ int qemu_strtoul(const char *nptr, const char **endptr, int base,
assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
+ *result = 0;
if (endptr) {
*endptr = nptr;
}
@@ -588,6 +596,7 @@ int qemu_strtoi64(const char *nptr, const char **endptr, int base,
assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
+ *result = 0;
if (endptr) {
*endptr = nptr;
}
@@ -613,6 +622,7 @@ int qemu_strtou64(const char *nptr, const char **endptr, int base,
assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
+ *result = 0;
if (endptr) {
*endptr = nptr;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 10/11] cutils: Improve qemu_strtod* error paths
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
` (8 preceding siblings ...)
2023-05-08 20:03 ` [PATCH 09/11] cutils: Set value in all integral qemu_strto* " Eric Blake
@ 2023-05-08 20:03 ` Eric Blake
2023-05-08 20:03 ` [PATCH 11/11] cutils: Improve qemu_strtosz handling of fractions Eric Blake
2023-05-09 17:55 ` [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Hanna Czenczek
11 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-08 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: hreitz
Previous patches changed all integral qemu_strto*() error paths to
guarantee that *value is never left uninitialized. Do likewise for
qemu_strtod. Also, tighten qemu_strtod_finite() to never return a
non-finite value (prior to this patch, we were rejecting "inf" with
-EINVAL and unspecified result 0.0, but failing "9e999" with -ERANGE
and HUGE_VAL - which is infinite on IEEE machines - despite our
function claiming to recognize only finite values).
Auditing callers, we have no external callers of qemu_strtod, and
among the callers of qemu_strtod_finite:
- qapi/qobject-input-visitor.c:qobject_input_type_number_keyval() and
qapi/string-input-visitor.c:parse_type_number() which reject all
errors (does not matter what we store)
- utils/cutils.c:do_strtosz() incorrectly assumes that *endptr points
to '.' on all failures (that is, it is not distinguishing between
EINVAL and ERANGE; and therefore still does the WRONG THING for
"9.9e999". The change here does not fix that (a later patch will
tackle this more systematically), but at least the value of endptr
is less likely to be out of bounds on overflow
- our testsuite, which we can update to match what we document
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/unit/test-cutils.c | 57 +++++++++++++++++++++++++---------------
util/cutils.c | 32 +++++++++++++---------
2 files changed, 55 insertions(+), 34 deletions(-)
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 2cb33e41ae4..f781997aef7 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -2105,6 +2105,7 @@ static void test_qemu_strtod_einval(void)
err = qemu_strtod(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
/* NULL */
@@ -2113,7 +2114,8 @@ static void test_qemu_strtod_einval(void)
res = 999;
err = qemu_strtod(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_null(endptr);
/* not recognizable */
@@ -2123,6 +2125,7 @@ static void test_qemu_strtod_einval(void)
err = qemu_strtod(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
}
@@ -2309,7 +2312,8 @@ static void test_qemu_strtod_finite_einval(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
/* NULL */
@@ -2318,7 +2322,8 @@ static void test_qemu_strtod_finite_einval(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_null(endptr);
/* not recognizable */
@@ -2327,7 +2332,8 @@ static void test_qemu_strtod_finite_einval(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
}
@@ -2338,24 +2344,26 @@ static void test_qemu_strtod_finite_erange(void)
int err;
double res;
- /* overflow */
+ /* overflow turns into EINVAL */
str = "9e999";
endptr = NULL;
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
- g_assert_cmpint(err, ==, -ERANGE);
- g_assert_cmpfloat(res, ==, HUGE_VAL);
- g_assert_true(endptr == str + 5);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
+ g_assert_true(endptr == str);
str = "-9e+999";
endptr = NULL;
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
- g_assert_cmpint(err, ==, -ERANGE);
- g_assert_cmpfloat(res, ==, -HUGE_VAL);
- g_assert_true(endptr == str + 7);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
+ g_assert_true(endptr == str);
- /* underflow */
+ /* underflow is still possible */
str = "-9e-999";
endptr = NULL;
res = 999;
@@ -2380,7 +2388,8 @@ static void test_qemu_strtod_finite_nonfinite(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
str = "-infinity";
@@ -2388,7 +2397,8 @@ static void test_qemu_strtod_finite_nonfinite(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
/* not a number */
@@ -2397,7 +2407,8 @@ static void test_qemu_strtod_finite_nonfinite(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
}
@@ -2421,7 +2432,8 @@ static void test_qemu_strtod_finite_trailing(void)
res = 999;
err = qemu_strtod_finite(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 1.0);
+ g_assert_false(signbit(res));
/* trailing e is not an exponent */
str = ".5e";
@@ -2436,7 +2448,8 @@ static void test_qemu_strtod_finite_trailing(void)
res = 999;
err = qemu_strtod_finite(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.5);
+ g_assert_false(signbit(res));
/* trailing ( not part of long NaN */
str = "nan(";
@@ -2444,14 +2457,16 @@ static void test_qemu_strtod_finite_trailing(void)
res = 999;
err = qemu_strtod_finite(str, &endptr, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
g_assert_true(endptr == str);
endptr = NULL;
res = 999;
err = qemu_strtod_finite(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
- g_assert_cmpfloat(res, ==, 999.0);
+ g_assert_cmpfloat(res, ==, 0.0);
+ g_assert_false(signbit(res));
}
static void test_qemu_strtosz_simple(void)
@@ -3063,8 +3078,8 @@ static void test_qemu_strtosz_trailing(void)
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 */);
+ g_assert_cmpuint(res, ==, 1 /* FIXME EiB * 1.5 */);
+ g_assert(endptr == str + 2 /* FIXME + 4 */);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
diff --git a/util/cutils.c b/util/cutils.c
index 83948926ec9..0e056a27a44 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -649,12 +649,13 @@ int qemu_strtou64(const char *nptr, const char **endptr, int base,
*
* @nptr may be null, and no conversion is performed then.
*
- * If no conversion is performed, store @nptr in *@endptr and return
- * -EINVAL.
+ * If no conversion is performed, store @nptr in *@endptr, +0.0 in
+ * @result, and return -EINVAL.
*
* If @endptr is null, and the string isn't fully converted, return
- * -EINVAL. This is the case when the pointer that would be stored in
- * a non-null @endptr points to a character other than '\0'.
+ * -EINVAL with @result set to the parsed value. This is the case
+ * when the pointer that would be stored in a non-null @endptr points
+ * to a character other than '\0'.
*
* If the conversion overflows, store +/-HUGE_VAL in @result, depending
* on the sign, and return -ERANGE.
@@ -669,6 +670,7 @@ int qemu_strtod(const char *nptr, const char **endptr, double *result)
char *ep;
if (!nptr) {
+ *result = 0.0;
if (endptr) {
*endptr = nptr;
}
@@ -683,24 +685,28 @@ int qemu_strtod(const char *nptr, const char **endptr, double *result)
/**
* Convert string @nptr to a finite double.
*
- * Works like qemu_strtod(), except that "NaN" and "inf" are rejected
- * with -EINVAL and no conversion is performed.
+ * Works like qemu_strtod(), except that "NaN", "inf", and strings
+ * that cause ERANGE overflow errors are rejected with -EINVAL as if
+ * no conversion is performed, storing 0.0 into @result regardless of
+ * any sign. -ERANGE failures for underflow still preserve the parsed
+ * sign.
*/
int qemu_strtod_finite(const char *nptr, const char **endptr, double *result)
{
- double tmp;
+ const char *tmp;
int ret;
- ret = qemu_strtod(nptr, endptr, &tmp);
- if (!ret && !isfinite(tmp)) {
+ ret = qemu_strtod(nptr, &tmp, result);
+ if (!isfinite(*result)) {
if (endptr) {
*endptr = nptr;
}
+ *result = 0.0;
+ ret = -EINVAL;
+ } else if (endptr) {
+ *endptr = tmp;
+ } else if (*tmp) {
ret = -EINVAL;
- }
-
- if (ret != -EINVAL) {
- *result = tmp;
}
return ret;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 11/11] cutils: Improve qemu_strtosz handling of fractions
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
` (9 preceding siblings ...)
2023-05-08 20:03 ` [PATCH 10/11] cutils: Improve qemu_strtod* " Eric Blake
@ 2023-05-08 20:03 ` Eric Blake
2023-05-08 21:21 ` Eric Blake
2023-05-09 17:54 ` Hanna Czenczek
2023-05-09 17:55 ` [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Hanna Czenczek
11 siblings, 2 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-08 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: hreitz
We have several limitations and bugs worth fixing; they are
inter-related enough that it is not worth splitting this patch into
smaller pieces:
* ".5k" should work to specify 512, just as "0.5k" does
* "1.9999k" and "1." + "9"*50 + "k" should both produce the same
result of 2048 after rounding
* "1." + "0"*350 + "1B" should not be treated the same as "1.0B";
underflow in the fraction should not be lost
* "7.99e99" and "7.99e999" look similar, but our code was doing a
read-out-of-bounds on the latter because it was not expecting ERANGE
due to overflow. While we document that scientific notation is not
supported, and the previous patch actually fixed
qemu_strtod_finite() to no longer return ERANGE overflows, it is
easier to pre-filter than to try and determine after the fact if
strtod() consumed more than we wanted. Note that this is a
low-level semantic change (when endptr is not NULL, we can now
successfully parse with a scale of 'E' and then report trailing
junk, instead of failing outright with EINVAL); but an earlier
commit already argued that this is not a high-level semantic change
since the only caller passing in a non-NULL endptr also checks that
the tail is whitespace-only.
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1629
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/unit/test-cutils.c | 51 +++++++++++------------
util/cutils.c | 89 ++++++++++++++++++++++++++++------------
2 files changed, 88 insertions(+), 52 deletions(-)
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index f781997aef7..1fb9d5323ab 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -2693,14 +2693,14 @@ static void test_qemu_strtosz_float(void)
g_assert_cmpuint(res, ==, 1024);
g_assert_true(endptr == str + 4);
- /* FIXME An empty fraction head should be tolerated */
+ /* An empty fraction head is tolerated */
str = " .5k";
endptr = str;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
- g_assert_cmpint(err, ==, -EINVAL /* FIXME 0 */);
- g_assert_cmpuint(res, ==, 0 /* FIXME 512 */);
- g_assert_true(endptr == str /* FIXME + 4 */);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpuint(res, ==, 512);
+ g_assert_true(endptr == str + 4);
/* For convenience, we permit values that are not byte-exact */
str = "12.345M";
@@ -2711,16 +2711,16 @@ static void test_qemu_strtosz_float(void)
g_assert_cmpuint(res, ==, (uint64_t) (12.345 * MiB + 0.5));
g_assert_true(endptr == str + 7);
- /* FIXME Fraction tail should round correctly */
+ /* Fraction tail can round up */
str = "1.9999999999999999999999999999999999999999999999999999k";
endptr = str;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpint(res, ==, 1024 /* FIXME 2048 */);
+ g_assert_cmpuint(res, ==, 2048);
g_assert_true(endptr == str + 55);
- /* FIXME ERANGE underflow in the fraction tail should not matter for 'k' */
+ /* ERANGE underflow in the fraction tail does not matter for 'k' */
str = "1."
"00000000000000000000000000000000000000000000000000"
"00000000000000000000000000000000000000000000000000"
@@ -2734,7 +2734,7 @@ static void test_qemu_strtosz_float(void)
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
g_assert_cmpint(err, ==, 0);
- g_assert_cmpuint(res, ==, 1 /* FIXME 1024 */);
+ g_assert_cmpuint(res, ==, 1024);
g_assert_true(endptr == str + 354);
}
@@ -2826,16 +2826,16 @@ static void test_qemu_strtosz_invalid(void)
g_assert_cmpuint(res, ==, 0);
g_assert_true(endptr == str);
- /* FIXME Fraction tail can cause ERANGE overflow */
+ /* Fraction tail can cause ERANGE overflow */
str = "15.9999999999999999999999999999999999999999999999999999E";
endptr = str;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
- g_assert_cmpint(err, ==, 0 /* FIXME -ERANGE */);
- g_assert_cmpuint(res, ==, 15ULL * EiB /* FIXME 0 */);
- g_assert_true(endptr == str + 56 /* FIXME str */);
+ g_assert_cmpint(err, ==, -ERANGE);
+ g_assert_cmpuint(res, ==, 0);
+ g_assert_true(endptr == str + 56);
- /* FIXME ERANGE underflow in the fraction tail should matter for 'B' */
+ /* ERANGE underflow in the fraction tail matters for 'B' */
str = "1."
"00000000000000000000000000000000000000000000000000"
"00000000000000000000000000000000000000000000000000"
@@ -2848,9 +2848,9 @@ static void test_qemu_strtosz_invalid(void)
endptr = str;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
- g_assert_cmpint(err, ==, 0 /* FIXME -EINVAL */);
- g_assert_cmpuint(res, ==, 1 /* FIXME 0 */);
- g_assert_true(endptr == str + 354 /* FIXME str */);
+ g_assert_cmpint(err, ==, -EINVAL);
+ g_assert_cmpuint(res, ==, 0);
+ g_assert_true(endptr == str);
/* No hex fractions */
str = "0x1.8k";
@@ -3045,14 +3045,14 @@ static void test_qemu_strtosz_trailing(void)
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpuint(res, ==, 0);
- /* FIXME should stop parse after 'e'. No floating point exponents */
+ /* Parse stops at 'e', which is not a floating point exponent */
str = "1.5e1k";
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
- g_assert_cmpint(err, ==, -EINVAL /* FIXME 0 */);
- g_assert_cmpuint(res, ==, 0 /* FIXME EiB * 1.5 */);
- g_assert_true(endptr == str /* FIXME + 4 */);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpuint(res, ==, EiB * 1.5);
+ g_assert_true(endptr == str + 4);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
@@ -3063,23 +3063,22 @@ static void test_qemu_strtosz_trailing(void)
endptr = NULL;
res = 0xbaadf00d;
err = qemu_strtosz(str, &endptr, &res);
- g_assert_cmpint(err, ==, -EINVAL /* FIXME 0 */);
- g_assert_cmpuint(res, ==, 0 /* FIXME EiB * 1.5 */);
- g_assert_true(endptr == str /* FIXME + 4 */);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmpuint(res, ==, EiB * 1.5);
+ g_assert_true(endptr == str + 4);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpuint(res, ==, 0);
- /* 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, ==, 1 /* FIXME EiB * 1.5 */);
- g_assert(endptr == str + 2 /* FIXME + 4 */);
+ g_assert_cmpuint(res, ==, EiB * 1.5);
+ g_assert_true(endptr == str + 4);
res = 0xbaadf00d;
err = qemu_strtosz(str, NULL, &res);
diff --git a/util/cutils.c b/util/cutils.c
index 0e056a27a44..d1dfbc69d16 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -194,14 +194,17 @@ static int64_t suffix_mul(char suffix, int64_t unit)
* - 12345 - decimal, scale determined by @default_suffix and @unit
* - 12345{bBkKmMgGtTpPeE} - decimal, scale determined by suffix and @unit
* - 12345.678{kKmMgGtTpPeE} - decimal, scale determined by suffix, and
- * fractional portion is truncated to byte
+ * fractional portion is truncated to byte, either side of . may be empty
* - 0x7fEE - hexadecimal, unit determined by @default_suffix
*
* The following are intentionally not supported
- * - hex with scaling suffix, such as 0x20M
- * - octal, such as 08
- * - fractional hex, such as 0x1.8
- * - floating point exponents, such as 1e3
+ * - hex with scaling suffix, such as 0x20M (0x1b is 27, not 1)
+ * - octal, such as 08 (parsed as decimal instead)
+ * - binary, such as 0b1000 (parsed as 0b with trailing garbage "1000")
+ * - fractional hex, such as 0x1.8 (parsed as 0 with trailing garbage "x1.8")
+ * - floating point exponents, such as 1e3 (parsed as 1e with trailing
+ * garbage "3") or 0x1p3 (parsed as 1 with trailing garbage "p3")
+ * - non-finite values, such as inf or NaN
*
* The end pointer will be returned in *end, if not NULL. If there is
* no fraction, the input can be decimal or hexadecimal; if there is a
@@ -220,22 +223,17 @@ static int do_strtosz(const char *nptr, const char **end,
uint64_t *result)
{
int retval;
- const char *endptr, *f;
+ const char *endptr;
unsigned char c;
- uint64_t val, valf = 0;
+ uint64_t val = 0, valf = 0;
int64_t mul;
/* Parse integral portion as decimal. */
retval = qemu_strtou64(nptr, &endptr, 10, &val);
- if (retval) {
+ if (retval == -ERANGE || !nptr) {
goto out;
}
- if (memchr(nptr, '-', endptr - nptr) != NULL) {
- endptr = nptr;
- retval = -EINVAL;
- goto out;
- }
- if (val == 0 && (*endptr == 'x' || *endptr == 'X')) {
+ if (retval == 0 && val == 0 && (*endptr == 'x' || *endptr == 'X')) {
/* Input looks like hex; reparse, and insist on no fraction or suffix. */
retval = qemu_strtou64(nptr, &endptr, 16, &val);
if (retval) {
@@ -246,27 +244,66 @@ static int do_strtosz(const char *nptr, const char **end,
retval = -EINVAL;
goto out;
}
- } else if (*endptr == '.') {
+ } else if (*endptr == '.' || (endptr == nptr && strchr(nptr, '.'))) {
/*
* Input looks like a fraction. Make sure even 1.k works
- * without fractional digits. If we see an exponent, treat
- * the entire input as invalid instead.
+ * without fractional digits. strtod tries to treat 'e' as an
+ * exponent, but we want to treat it as a scaling suffix;
+ * doing this requires modifying a copy of the fraction.
*/
- double fraction;
+ double fraction = 0.0;
- f = endptr;
- retval = qemu_strtod_finite(f, &endptr, &fraction);
- if (retval) {
+ if (retval == 0 && *endptr == '.' && !isdigit(endptr[1])) {
+ /* If we got here, we parsed at least one digit already. */
endptr++;
- } else if (memchr(f, 'e', endptr - f) || memchr(f, 'E', endptr - f)) {
- endptr = nptr;
- retval = -EINVAL;
- goto out;
} else {
- /* Extract into a 64-bit fixed-point fraction. */
+ char *e;
+ const char *tail;
+ g_autofree char *copy = g_strdup(endptr);
+
+ e = strchr(copy, 'e');
+ if (e) {
+ *e = '\0';
+ }
+ e = strchr(copy, 'E');
+ if (e) {
+ *e = '\0';
+ }
+ /*
+ * If this is a floating point, we are guaranteed that '.'
+ * appears before any possible digits in copy. If it is
+ * not a floating point, strtod will fail. Either way,
+ * there is now no exponent in copy, so if it parses, we
+ * know 0.0 <= abs(result) <= 1.0 (after rounding), and
+ * ERANGE is only possible on underflow which is okay.
+ */
+ retval = qemu_strtod_finite(copy, &tail, &fraction);
+ endptr += tail - copy;
+ }
+
+ /* Extract into a 64-bit fixed-point fraction. */
+ if (fraction == 1.0) {
+ if (val == UINT64_MAX) {
+ retval = -ERANGE;
+ goto out;
+ }
+ val++;
+ } else if (retval == -ERANGE) {
+ /* See comments above about underflow */
+ valf = 1;
+ retval = 0;
+ } else {
valf = (uint64_t)(fraction * 0x1p64);
}
}
+ if (retval) {
+ goto out;
+ }
+ if (memchr(nptr, '-', endptr - nptr) != NULL) {
+ endptr = nptr;
+ retval = -EINVAL;
+ goto out;
+ }
c = *endptr;
mul = suffix_mul(c, unit);
if (mul > 0) {
--
2.40.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 11/11] cutils: Improve qemu_strtosz handling of fractions
2023-05-08 20:03 ` [PATCH 11/11] cutils: Improve qemu_strtosz handling of fractions Eric Blake
@ 2023-05-08 21:21 ` Eric Blake
2023-05-09 17:54 ` Hanna Czenczek
1 sibling, 0 replies; 25+ messages in thread
From: Eric Blake @ 2023-05-08 21:21 UTC (permalink / raw)
To: qemu-devel; +Cc: hreitz
On Mon, May 08, 2023 at 03:03:43PM -0500, Eric Blake wrote:
> We have several limitations and bugs worth fixing; they are
> inter-related enough that it is not worth splitting this patch into
> smaller pieces:
>
> * ".5k" should work to specify 512, just as "0.5k" does
> * "1.9999k" and "1." + "9"*50 + "k" should both produce the same
> result of 2048 after rounding
> * "1." + "0"*350 + "1B" should not be treated the same as "1.0B";
> underflow in the fraction should not be lost
> * "7.99e99" and "7.99e999" look similar, but our code was doing a
> read-out-of-bounds on the latter because it was not expecting ERANGE
> due to overflow. While we document that scientific notation is not
> supported, and the previous patch actually fixed
> qemu_strtod_finite() to no longer return ERANGE overflows, it is
> easier to pre-filter than to try and determine after the fact if
> strtod() consumed more than we wanted. Note that this is a
> low-level semantic change (when endptr is not NULL, we can now
> successfully parse with a scale of 'E' and then report trailing
> junk, instead of failing outright with EINVAL); but an earlier
> commit already argued that this is not a high-level semantic change
> since the only caller passing in a non-NULL endptr also checks that
> the tail is whitespace-only.
>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1629
Also,
Fixes: cf923b78 ("utils: Improve qemu_strtosz() to have 64 bits of precision", 6.0.0)
Fixes: 7625a1ed ("utils: Use fixed-point arithmetic in qemu_strtosz", 6.0.0)
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> tests/unit/test-cutils.c | 51 +++++++++++------------
> util/cutils.c | 89 ++++++++++++++++++++++++++++------------
> 2 files changed, 88 insertions(+), 52 deletions(-)
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 11/11] cutils: Improve qemu_strtosz handling of fractions
2023-05-08 20:03 ` [PATCH 11/11] cutils: Improve qemu_strtosz handling of fractions Eric Blake
2023-05-08 21:21 ` Eric Blake
@ 2023-05-09 17:54 ` Hanna Czenczek
2023-05-09 21:28 ` Eric Blake
1 sibling, 1 reply; 25+ messages in thread
From: Hanna Czenczek @ 2023-05-09 17:54 UTC (permalink / raw)
To: Eric Blake, qemu-devel
On 08.05.23 22:03, Eric Blake wrote:
> We have several limitations and bugs worth fixing; they are
> inter-related enough that it is not worth splitting this patch into
> smaller pieces:
>
> * ".5k" should work to specify 512, just as "0.5k" does
> * "1.9999k" and "1." + "9"*50 + "k" should both produce the same
> result of 2048 after rounding
> * "1." + "0"*350 + "1B" should not be treated the same as "1.0B";
> underflow in the fraction should not be lost
> * "7.99e99" and "7.99e999" look similar, but our code was doing a
> read-out-of-bounds on the latter because it was not expecting ERANGE
> due to overflow. While we document that scientific notation is not
> supported, and the previous patch actually fixed
> qemu_strtod_finite() to no longer return ERANGE overflows, it is
> easier to pre-filter than to try and determine after the fact if
> strtod() consumed more than we wanted. Note that this is a
> low-level semantic change (when endptr is not NULL, we can now
> successfully parse with a scale of 'E' and then report trailing
> junk, instead of failing outright with EINVAL); but an earlier
> commit already argued that this is not a high-level semantic change
> since the only caller passing in a non-NULL endptr also checks that
> the tail is whitespace-only.
>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1629
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> tests/unit/test-cutils.c | 51 +++++++++++------------
> util/cutils.c | 89 ++++++++++++++++++++++++++++------------
> 2 files changed, 88 insertions(+), 52 deletions(-)
[...]
> diff --git a/util/cutils.c b/util/cutils.c
> index 0e056a27a44..d1dfbc69d16 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
[...]
> @@ -246,27 +244,66 @@ static int do_strtosz(const char *nptr, const char **end,
> retval = -EINVAL;
> goto out;
> }
> - } else if (*endptr == '.') {
> + } else if (*endptr == '.' || (endptr == nptr && strchr(nptr, '.'))) {
What case is there where we have a fraction but *endptr != '.'?
> /*
> * Input looks like a fraction. Make sure even 1.k works
> - * without fractional digits. If we see an exponent, treat
> - * the entire input as invalid instead.
> + * without fractional digits. strtod tries to treat 'e' as an
> + * exponent, but we want to treat it as a scaling suffix;
> + * doing this requires modifying a copy of the fraction.
> */
> - double fraction;
> + double fraction = 0.0;
>
> - f = endptr;
> - retval = qemu_strtod_finite(f, &endptr, &fraction);
> - if (retval) {
> + if (retval == 0 && *endptr == '.' && !isdigit(endptr[1])) {
> + /* If we got here, we parsed at least one digit already. */
> endptr++;
> - } else if (memchr(f, 'e', endptr - f) || memchr(f, 'E', endptr - f)) {
> - endptr = nptr;
> - retval = -EINVAL;
> - goto out;
> } else {
> - /* Extract into a 64-bit fixed-point fraction. */
> + char *e;
> + const char *tail;
> + g_autofree char *copy = g_strdup(endptr);
> +
> + e = strchr(copy, 'e');
> + if (e) {
> + *e = '\0';
> + }
> + e = strchr(copy, 'E');
> + if (e) {
> + *e = '\0';
> + }
> + /*
> + * If this is a floating point, we are guaranteed that '.'
> + * appears before any possible digits in copy. If it is
> + * not a floating point, strtod will fail. Either way,
> + * there is now no exponent in copy, so if it parses, we
> + * know 0.0 <= abs(result) <= 1.0 (after rounding), and
> + * ERANGE is only possible on underflow which is okay.
> + */
> + retval = qemu_strtod_finite(copy, &tail, &fraction);
> + endptr += tail - copy;
> + }
> +
> + /* Extract into a 64-bit fixed-point fraction. */
> + if (fraction == 1.0) {
> + if (val == UINT64_MAX) {
> + retval = -ERANGE;
> + goto out;
> + }
> + val++;
> + } else if (retval == -ERANGE) {
> + /* See comments above about underflow */
> + valf = 1;
It doesn’t really matter because even an EiB is just 2^60, and so 1 EiB
* 2^-64 (the resolution of our fractional part) is still less than 1, but:
DBL_MIN * 0x1p64 is 2^-(1022-64) == 2^-958, i.e. much less than 1, so
I’d set valf to 0 here.
(If you put “.00000000000000000001” into this, there won’t be an
underflow, but the value is so small that valf ends up 0. But if you
put `.$(yes 0 | head -n 307 | tr -d '\n')1` into this, there will be an
underflow, setting valf to 1, even though the value is smaller.)
Hanna
> + retval = 0;
> + } else {
> valf = (uint64_t)(fraction * 0x1p64);
> }
> }
> + if (retval) {
> + goto out;
> + }
> + if (memchr(nptr, '-', endptr - nptr) != NULL) {
> + endptr = nptr;
> + retval = -EINVAL;
> + goto out;
> + }
> c = *endptr;
> mul = suffix_mul(c, unit);
> if (mul > 0) {
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 11/11] cutils: Improve qemu_strtosz handling of fractions
2023-05-09 17:54 ` Hanna Czenczek
@ 2023-05-09 21:28 ` Eric Blake
2023-05-10 7:46 ` Hanna Czenczek
0 siblings, 1 reply; 25+ messages in thread
From: Eric Blake @ 2023-05-09 21:28 UTC (permalink / raw)
To: Hanna Czenczek; +Cc: qemu-devel
On Tue, May 09, 2023 at 07:54:30PM +0200, Hanna Czenczek wrote:
> On 08.05.23 22:03, Eric Blake wrote:
> > We have several limitations and bugs worth fixing; they are
> > inter-related enough that it is not worth splitting this patch into
> > smaller pieces:
> >
> > * ".5k" should work to specify 512, just as "0.5k" does
> > * "1.9999k" and "1." + "9"*50 + "k" should both produce the same
> > result of 2048 after rounding
> > * "1." + "0"*350 + "1B" should not be treated the same as "1.0B";
> > underflow in the fraction should not be lost
> > * "7.99e99" and "7.99e999" look similar, but our code was doing a
> > read-out-of-bounds on the latter because it was not expecting ERANGE
> > due to overflow. While we document that scientific notation is not
> > supported, and the previous patch actually fixed
> > qemu_strtod_finite() to no longer return ERANGE overflows, it is
> > easier to pre-filter than to try and determine after the fact if
> > strtod() consumed more than we wanted. Note that this is a
> > low-level semantic change (when endptr is not NULL, we can now
> > successfully parse with a scale of 'E' and then report trailing
> > junk, instead of failing outright with EINVAL); but an earlier
> > commit already argued that this is not a high-level semantic change
> > since the only caller passing in a non-NULL endptr also checks that
> > the tail is whitespace-only.
> >
> > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1629
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> > ---
> > tests/unit/test-cutils.c | 51 +++++++++++------------
> > util/cutils.c | 89 ++++++++++++++++++++++++++++------------
> > 2 files changed, 88 insertions(+), 52 deletions(-)
>
> [...]
>
> > diff --git a/util/cutils.c b/util/cutils.c
> > index 0e056a27a44..d1dfbc69d16 100644
> > --- a/util/cutils.c
> > +++ b/util/cutils.c
>
> [...]
>
> > @@ -246,27 +244,66 @@ static int do_strtosz(const char *nptr, const char **end,
> > retval = -EINVAL;
> > goto out;
> > }
> > - } else if (*endptr == '.') {
> > + } else if (*endptr == '.' || (endptr == nptr && strchr(nptr, '.'))) {
>
> What case is there where we have a fraction but *endptr != '.'?
Bigger context:
result = qemu_strtou64(nptr, &endptr, 10, &val);
// at this point, result is one of:
// a. 0 - we parsed a decimal string, endptr points to any slop
// b. -EINVAL - we could not recognize a decimal string: multiple reasons
// b.1. nptr was NULL (endptr is NULL)
// b.2. nptr was "" or otherwise whitespace only (endptr is nptr)
// b.3. the first non-whitespace in nptr was not a sign or digit (endptr is nptr)
// c. -ERANGE - we saw a decimal string, but it didn't fit in uint64 (endptr is
// past first digit)
if (retval == -ERANGE || !nptr) {
// filter out c. and b.1
goto out;
}
if (retval == 0 && val == 0 && (*endptr == 'x' || *endptr == 'X')) {
// a, where we must decipher between "0x", "00x", "0xjunk", "0x1", ...
// not changed by this patch, and where we give -EINVAL if we see any trailing
// slop like "0x1." or "0x1p"
} else if (*endptr == '.' || (endptr == nptr && strchr(nptr, '.'))) {
// The left half is possible in both a. (such as "1.5k")
// and b.3. when '.' was the first slop byte (such as ".5k")
// The right half is possible only for b.3 when '.' was not the first slop
// (needed for covering " +.5k")
// At this point, b.2. has been filtered out
...
>
> > /*
> > * Input looks like a fraction. Make sure even 1.k works
> > - * without fractional digits. If we see an exponent, treat
> > - * the entire input as invalid instead.
> > + * without fractional digits. strtod tries to treat 'e' as an
> > + * exponent, but we want to treat it as a scaling suffix;
> > + * doing this requires modifying a copy of the fraction.
> > */
> > - double fraction;
> > + double fraction = 0.0;
> >
> > - f = endptr;
> > - retval = qemu_strtod_finite(f, &endptr, &fraction);
> > - if (retval) {
> > + if (retval == 0 && *endptr == '.' && !isdigit(endptr[1])) {
> > + /* If we got here, we parsed at least one digit already. */
> > endptr++;
The 'retval == 0' check could equally be written 'endptr > nptr' (the
two are synonymous based on the conditions of a.; we cannot get here
under b.3); the '*endptr == '.' is necessary so that if nptr=="1junk",
we use 'j' as the scaling suffix rather than trying to skip past a
non-present '.'; and the '!isdigit(endptr[1])' is necessary so that
"1.k" does not result in us trying to call strtod(".k") which would
fail for an unexpected EINVAL. Basically, this branch handles all
cases where we've seen at least one digit and the only thing between
digits and a possible scaling suffix is a single '.', so strtod is not
worth using.
> > - } else if (memchr(f, 'e', endptr - f) || memchr(f, 'E', endptr - f)) {
> > - endptr = nptr;
> > - retval = -EINVAL;
> > - goto out;
> > } else {
> > - /* Extract into a 64-bit fixed-point fraction. */
> > + char *e;
> > + const char *tail;
> > + g_autofree char *copy = g_strdup(endptr);
> > +
If we get into this branch, we could be in condition a. (such as
"1.1k" where endptr is ".1k") or in b.3 (such as ".5k" where endptr is
".5k", but also thinks like " junk." where endptr is " junk." or even
".k"). But we've already proven we don't need to worry about "0x1p1"
(filtered above in the hex code), and at this point we strip all
exponents (if endptr is ".9e999", copy is ".9")...
> > + e = strchr(copy, 'e');
> > + if (e) {
> > + *e = '\0';
> > + }
> > + e = strchr(copy, 'E');
> > + if (e) {
> > + *e = '\0';
> > + }
> > + /*
> > + * If this is a floating point, we are guaranteed that '.'
> > + * appears before any possible digits in copy. If it is
> > + * not a floating point, strtod will fail. Either way,
> > + * there is now no exponent in copy, so if it parses, we
> > + * know 0.0 <= abs(result) <= 1.0 (after rounding), and
> > + * ERANGE is only possible on underflow which is okay.
> > + */
> > + retval = qemu_strtod_finite(copy, &tail, &fraction);
...so that by the time we do try qemu_strtod_finite(), it is either a
valid floating point fraction with at least one digit and no exponent
and possibly some slop (such as ".5k" or " +.5" - will produce retval
= 0 or -ERANGE for underflow, based on the previous patch to
qemu_strtod_finite) or complete junk with retval = -EINVAL where there
was a '.' but other characters appeared first (such as " junk.") or
where there are no digits but also no scaling suffix (such as ". ";
hmm, looks like I could get more coverage if I add ". " and ".k" to my
unit tests in v2).
> > + endptr += tail - copy;
> > + }
> > +
> > + /* Extract into a 64-bit fixed-point fraction. */
> > + if (fraction == 1.0) {
> > + if (val == UINT64_MAX) {
> > + retval = -ERANGE;
> > + goto out;
> > + }
> > + val++;
> > + } else if (retval == -ERANGE) {
> > + /* See comments above about underflow */
> > + valf = 1;
>
> It doesn’t really matter because even an EiB is just 2^60, and so 1 EiB *
> 2^-64 (the resolution of our fractional part) is still less than 1, but:
>
> DBL_MIN * 0x1p64 is 2^-(1022-64) == 2^-958, i.e. much less than 1, so I’d
> set valf to 0 here.
>
> (If you put “.00000000000000000001” into this, there won’t be an underflow,
> but the value is so small that valf ends up 0. But if you put `.$(yes 0 |
> head -n 307 | tr -d '\n')1` into this, there will be an underflow, setting
> valf to 1, even though the value is smaller.)
Oh, good point. I was trying to say that "1.000B" (for any amount of
zeroes) is okay (all zeroes in the fraction is needless typing but not
an ambiguous value regardless of rounding), while "1.0001B" (for any
amount of zeroes) is not (you can't request a non-zero fraction
without a scale larger than bytes, even if the fraction you do request
rounds to zero at your chosen scale). But you have come up with a
counter-case where I didn't quite achieve that.
But I think the solution to that is not to treat underflow as valf =
0, but rather to alter this snippet:
- valf = (uint64_t)(fraction * 0x1p64);
+ /*
+ * If fraction was non-zero, add slop small enough that it doesn't
+ * impact rounding, but does let us reject "1..00000000000000000001B".
+ */
+ valf = (uint64_t)(fraction * 0x1p64) | !fraction;
so that between the ERANGE branch and this slop, valf is guaranteed
non-zero if fraction contained any non-zero digits. It looks like I
need to add yet another unit test before posting v2.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 11/11] cutils: Improve qemu_strtosz handling of fractions
2023-05-09 21:28 ` Eric Blake
@ 2023-05-10 7:46 ` Hanna Czenczek
2023-05-10 7:48 ` Hanna Czenczek
0 siblings, 1 reply; 25+ messages in thread
From: Hanna Czenczek @ 2023-05-10 7:46 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel
On 09.05.23 23:28, Eric Blake wrote:
> On Tue, May 09, 2023 at 07:54:30PM +0200, Hanna Czenczek wrote:
>> On 08.05.23 22:03, Eric Blake wrote:
>>> We have several limitations and bugs worth fixing; they are
>>> inter-related enough that it is not worth splitting this patch into
>>> smaller pieces:
>>>
>>> * ".5k" should work to specify 512, just as "0.5k" does
>>> * "1.9999k" and "1." + "9"*50 + "k" should both produce the same
>>> result of 2048 after rounding
>>> * "1." + "0"*350 + "1B" should not be treated the same as "1.0B";
>>> underflow in the fraction should not be lost
>>> * "7.99e99" and "7.99e999" look similar, but our code was doing a
>>> read-out-of-bounds on the latter because it was not expecting ERANGE
>>> due to overflow. While we document that scientific notation is not
>>> supported, and the previous patch actually fixed
>>> qemu_strtod_finite() to no longer return ERANGE overflows, it is
>>> easier to pre-filter than to try and determine after the fact if
>>> strtod() consumed more than we wanted. Note that this is a
>>> low-level semantic change (when endptr is not NULL, we can now
>>> successfully parse with a scale of 'E' and then report trailing
>>> junk, instead of failing outright with EINVAL); but an earlier
>>> commit already argued that this is not a high-level semantic change
>>> since the only caller passing in a non-NULL endptr also checks that
>>> the tail is whitespace-only.
>>>
>>> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1629
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>> ---
>>> tests/unit/test-cutils.c | 51 +++++++++++------------
>>> util/cutils.c | 89 ++++++++++++++++++++++++++++------------
>>> 2 files changed, 88 insertions(+), 52 deletions(-)
>> [...]
>>
>>> diff --git a/util/cutils.c b/util/cutils.c
>>> index 0e056a27a44..d1dfbc69d16 100644
>>> --- a/util/cutils.c
>>> +++ b/util/cutils.c
>> [...]
>>
>>> @@ -246,27 +244,66 @@ static int do_strtosz(const char *nptr, const char **end,
>>> retval = -EINVAL;
>>> goto out;
>>> }
>>> - } else if (*endptr == '.') {
>>> + } else if (*endptr == '.' || (endptr == nptr && strchr(nptr, '.'))) {
>> What case is there where we have a fraction but *endptr != '.'?
> Bigger context:
>
> result = qemu_strtou64(nptr, &endptr, 10, &val);
> // at this point, result is one of:
> // a. 0 - we parsed a decimal string, endptr points to any slop
> // b. -EINVAL - we could not recognize a decimal string: multiple reasons
> // b.1. nptr was NULL (endptr is NULL)
> // b.2. nptr was "" or otherwise whitespace only (endptr is nptr)
> // b.3. the first non-whitespace in nptr was not a sign or digit (endptr is nptr)
> // c. -ERANGE - we saw a decimal string, but it didn't fit in uint64 (endptr is
> // past first digit)
> if (retval == -ERANGE || !nptr) {
> // filter out c. and b.1
> goto out;
> }
> if (retval == 0 && val == 0 && (*endptr == 'x' || *endptr == 'X')) {
> // a, where we must decipher between "0x", "00x", "0xjunk", "0x1", ...
> // not changed by this patch, and where we give -EINVAL if we see any trailing
> // slop like "0x1." or "0x1p"
> } else if (*endptr == '.' || (endptr == nptr && strchr(nptr, '.'))) {
> // The left half is possible in both a. (such as "1.5k")
> // and b.3. when '.' was the first slop byte (such as ".5k")
> // The right half is possible only for b.3 when '.' was not the first slop
> // (needed for covering " +.5k")
Ah, I see. Yes, I couldn’t come up with parseable non-digit characters
(whitespace, '+'). Thanks, that makes sense!
> // At this point, b.2. has been filtered out
>
> ...
>
>>> /*
>>> * Input looks like a fraction. Make sure even 1.k works
>>> - * without fractional digits. If we see an exponent, treat
>>> - * the entire input as invalid instead.
>>> + * without fractional digits. strtod tries to treat 'e' as an
>>> + * exponent, but we want to treat it as a scaling suffix;
>>> + * doing this requires modifying a copy of the fraction.
>>> */
>>> - double fraction;
>>> + double fraction = 0.0;
>>>
>>> - f = endptr;
>>> - retval = qemu_strtod_finite(f, &endptr, &fraction);
>>> - if (retval) {
>>> + if (retval == 0 && *endptr == '.' && !isdigit(endptr[1])) {
>>> + /* If we got here, we parsed at least one digit already. */
>>> endptr++;
> The 'retval == 0' check could equally be written 'endptr > nptr' (the
> two are synonymous based on the conditions of a.; we cannot get here
> under b.3); the '*endptr == '.' is necessary so that if nptr=="1junk",
> we use 'j' as the scaling suffix rather than trying to skip past a
> non-present '.'; and the '!isdigit(endptr[1])' is necessary so that
> "1.k" does not result in us trying to call strtod(".k") which would
> fail for an unexpected EINVAL. Basically, this branch handles all
> cases where we've seen at least one digit and the only thing between
> digits and a possible scaling suffix is a single '.', so strtod is not
> worth using.
>
>>> - } else if (memchr(f, 'e', endptr - f) || memchr(f, 'E', endptr - f)) {
>>> - endptr = nptr;
>>> - retval = -EINVAL;
>>> - goto out;
>>> } else {
>>> - /* Extract into a 64-bit fixed-point fraction. */
>>> + char *e;
>>> + const char *tail;
>>> + g_autofree char *copy = g_strdup(endptr);
>>> +
> If we get into this branch, we could be in condition a. (such as
> "1.1k" where endptr is ".1k") or in b.3 (such as ".5k" where endptr is
> ".5k", but also thinks like " junk." where endptr is " junk." or even
> ".k"). But we've already proven we don't need to worry about "0x1p1"
> (filtered above in the hex code), and at this point we strip all
> exponents (if endptr is ".9e999", copy is ".9")...
>
>>> + e = strchr(copy, 'e');
>>> + if (e) {
>>> + *e = '\0';
>>> + }
>>> + e = strchr(copy, 'E');
>>> + if (e) {
>>> + *e = '\0';
>>> + }
>>> + /*
>>> + * If this is a floating point, we are guaranteed that '.'
>>> + * appears before any possible digits in copy. If it is
>>> + * not a floating point, strtod will fail. Either way,
>>> + * there is now no exponent in copy, so if it parses, we
>>> + * know 0.0 <= abs(result) <= 1.0 (after rounding), and
>>> + * ERANGE is only possible on underflow which is okay.
>>> + */
>>> + retval = qemu_strtod_finite(copy, &tail, &fraction);
> ...so that by the time we do try qemu_strtod_finite(), it is either a
> valid floating point fraction with at least one digit and no exponent
> and possibly some slop (such as ".5k" or " +.5" - will produce retval
> = 0 or -ERANGE for underflow, based on the previous patch to
> qemu_strtod_finite) or complete junk with retval = -EINVAL where there
> was a '.' but other characters appeared first (such as " junk.") or
> where there are no digits but also no scaling suffix (such as ". ";
> hmm, looks like I could get more coverage if I add ". " and ".k" to my
> unit tests in v2).
>
>>> + endptr += tail - copy;
>>> + }
>>> +
>>> + /* Extract into a 64-bit fixed-point fraction. */
>>> + if (fraction == 1.0) {
>>> + if (val == UINT64_MAX) {
>>> + retval = -ERANGE;
>>> + goto out;
>>> + }
>>> + val++;
>>> + } else if (retval == -ERANGE) {
>>> + /* See comments above about underflow */
>>> + valf = 1;
>> It doesn’t really matter because even an EiB is just 2^60, and so 1 EiB *
>> 2^-64 (the resolution of our fractional part) is still less than 1, but:
>>
>> DBL_MIN * 0x1p64 is 2^-(1022-64) == 2^-958, i.e. much less than 1, so I’d
>> set valf to 0 here.
>>
>> (If you put “.00000000000000000001” into this, there won’t be an underflow,
>> but the value is so small that valf ends up 0. But if you put `.$(yes 0 |
>> head -n 307 | tr -d '\n')1` into this, there will be an underflow, setting
>> valf to 1, even though the value is smaller.)
> Oh, good point. I was trying to say that "1.000B" (for any amount of
> zeroes) is okay (all zeroes in the fraction is needless typing but not
> an ambiguous value regardless of rounding), while "1.0001B" (for any
> amount of zeroes) is not (you can't request a non-zero fraction
> without a scale larger than bytes, even if the fraction you do request
> rounds to zero at your chosen scale). But you have come up with a
> counter-case where I didn't quite achieve that.
>
> But I think the solution to that is not to treat underflow as valf =
> 0, but rather to alter this snippet:
>
> - valf = (uint64_t)(fraction * 0x1p64);
> + /*
> + * If fraction was non-zero, add slop small enough that it doesn't
> + * impact rounding, but does let us reject "1..00000000000000000001B".
> + */
> + valf = (uint64_t)(fraction * 0x1p64) | !fraction;
>
> so that between the ERANGE branch and this slop, valf is guaranteed
> non-zero if fraction contained any non-zero digits.
I’d make it a logical || instead of |, but that sounds good, yes.
Hanna
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 11/11] cutils: Improve qemu_strtosz handling of fractions
2023-05-10 7:46 ` Hanna Czenczek
@ 2023-05-10 7:48 ` Hanna Czenczek
0 siblings, 0 replies; 25+ messages in thread
From: Hanna Czenczek @ 2023-05-10 7:48 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel
On 10.05.23 09:46, Hanna Czenczek wrote:
> On 09.05.23 23:28, Eric Blake wrote:
[...]
>> But I think the solution to that is not to treat underflow as valf =
>> 0, but rather to alter this snippet:
>>
>> - valf = (uint64_t)(fraction * 0x1p64);
>> + /*
>> + * If fraction was non-zero, add slop small enough that
>> it doesn't
>> + * impact rounding, but does let us reject
>> "1..00000000000000000001B".
>> + */
>> + valf = (uint64_t)(fraction * 0x1p64) | !fraction;
>>
>> so that between the ERANGE branch and this slop, valf is guaranteed
>> non-zero if fraction contained any non-zero digits.
>
> I’d make it a logical || instead of |, but that sounds good, yes.
Sent too soon – exactly when sending I realized that I’m very wrong.
Still, I wouldn’t just | the 1 on, and rather make take the LoC to make it
valf = (uint64_t)(fraction * 0x1p64);
if (!valf && fraction) {
/*
* If fraction was non-zero, add slop small enough that it doesn't
* impact rounding, but does let us reject "1..00000000000000000001B".
*/
valf = 1;
}
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
` (10 preceding siblings ...)
2023-05-08 20:03 ` [PATCH 11/11] cutils: Improve qemu_strtosz handling of fractions Eric Blake
@ 2023-05-09 17:55 ` Hanna Czenczek
11 siblings, 0 replies; 25+ messages in thread
From: Hanna Czenczek @ 2023-05-09 17:55 UTC (permalink / raw)
To: Eric Blake, qemu-devel
On 08.05.23 22:03, Eric Blake wrote:
> This series blew up in my face when Hanna first pointed me to
> https://gitlab.com/qemu-project/qemu/-/issues/1629
>
> Basically, 'qemu-img dd bs=9.9e999' killed a sanitized build because
> of a read-out-of-bounds (".9e999" parses as infinity, but qemu_strtosz
> wasn't expecting ERANGE failure).
>
> The overall diffstate is big, mainly because the unit tests needed a
> LOT of work before I felt comfortable tweaking semantics in something
> that is so essential to command-line and QMP parsing.
>
> Eric Blake (11):
> test-cutils: Avoid g_assert in unit tests
> test-cutils: Use g_assert_cmpuint where appropriate
> test-cutils: Test integral qemu_strto* value on failures
> test-cutils: Add coverage of qemu_strtod
> test-cutils: Prepare for upcoming semantic change in qemu_strtosz
> test-cutils: Add more coverage to qemu_strtosz
> numa: Check for qemu_strtosz_MiB error
> cutils: Set value in all qemu_strtosz* error paths
> cutils: Set value in all integral qemu_strto* error paths
> cutils: Improve qemu_strtod* error paths
> cutils: Improve qemu_strtosz handling of fractions
>
> hw/core/numa.c | 11 +-
> tests/unit/test-cutils.c | 1213 ++++++++++++++++++++++++++++++--------
> util/cutils.c | 180 ++++--
> 3 files changed, 1090 insertions(+), 314 deletions(-)
Patches 1 – 5, 7 – 10:
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
^ permalink raw reply [flat|nested] 25+ messages in thread