cutils: Fix wraparound parsing in qemu_strtoui
While we were matching 32-bit strtol in qemu_strtoi, our use of a
64-bit parse was leaking through for some inaccurate answers in
qemu_strtoui in comparison to a 32-bit strtoul (see the unit test for
examples). The comment for that function even described what we have
to do for a correct parse, but didn't implement it correctly: since
strtoull checks for overflow against the wrong values and then
negates, we have to temporarily undo negation before checking for
overflow against our desired value.
Our int wrappers would be a lot easier to write if libc had a
guaranteed 32-bit parser even on platforms with 64-bit long.
Whether we parse C2x binary strings like "0b1000" is currently up to
what libc does; our unit tests intentionally don't cover that at the
moment, though.
Fixes: 473a2a331e ("cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types", v2.12.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
CC: qemu-stable@nongnu.org
Message-Id: <20230522190441.64278-6-eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 30cb5f1..8d2e057 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -909,7 +909,7 @@
static void test_qemu_strtoui_wrap(void)
{
- /* FIXME - wraparound should be consistent with 32-bit strtoul */
+ /* wraparound is consistent with 32-bit strtoul */
const char *str = "-4294967295"; /* 1 mod 2^32 */
char f = 'X';
const char *endptr = &f;
@@ -918,8 +918,8 @@
err = qemu_strtoui(str, &endptr, 0, &res);
- g_assert_cmpint(err, ==, -ERANGE /* FIXME 0 */);
- g_assert_cmphex(res, ==, UINT_MAX /* FIXME 1 */);
+ g_assert_cmpint(err, ==, 0);
+ g_assert_cmphex(res, ==, 1);
g_assert_true(endptr == str + strlen(str));
}
@@ -978,13 +978,12 @@
g_assert_cmpuint(res, ==, UINT_MAX);
g_assert_true(endptr == str + strlen(str));
- /* FIXME - overflow should be consistent with 32-bit strtoul */
str = "0xfffffffffffffffe"; /* ULLONG_MAX - 1 (not UINT_MAX - 1) */
endptr = "somewhere";
res = 999;
err = qemu_strtoui(str, &endptr, 0, &res);
- g_assert_cmpint(err, ==, 0 /* FIXME -ERANGE */);
- g_assert_cmpuint(res, ==, UINT_MAX - 1 /* FIXME UINT_MAX */);
+ g_assert_cmpint(err, ==, -ERANGE);
+ g_assert_cmpuint(res, ==, UINT_MAX);
g_assert_true(endptr == str + strlen(str));
str = "0x10000000000000000"; /* 65 bits, 32-bit sign bit clear */
@@ -1019,21 +1018,20 @@
g_assert_cmpuint(res, ==, UINT_MAX);
g_assert_true(endptr == str + strlen(str));
- /* FIXME - overflow should be consistent with 32-bit strtoul */
str = "-18446744073709551615"; /* -UINT64_MAX (not -(-1)) */
endptr = "somewhere";
res = 999;
err = qemu_strtoui(str, &endptr, 0, &res);
- g_assert_cmpint(err, ==, 0 /* FIXME -ERANGE */);
- g_assert_cmpuint(res, ==, 1 /* FIXME UINT_MAX */);
+ g_assert_cmpint(err, ==, -ERANGE);
+ g_assert_cmpuint(res, ==, UINT_MAX);
g_assert_true(endptr == str + strlen(str));
str = "-0xffffffff00000002";
endptr = "somewhere";
res = 999;
err = qemu_strtoui(str, &endptr, 0, &res);
- g_assert_cmpint(err, ==, 0 /* FIXME -ERANGE */);
- g_assert_cmpuint(res, ==, UINT_MAX - 1 /* FIXME UINT_MAX */);
+ g_assert_cmpint(err, ==, -ERANGE);
+ g_assert_cmpuint(res, ==, UINT_MAX);
g_assert_true(endptr == str + strlen(str));
str = "-0x10000000000000000"; /* 65 bits, 32-bit sign bit clear */
diff --git a/util/cutils.c b/util/cutils.c
index 5887e74..9b6ce91 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -391,6 +391,9 @@
* and return -ERANGE.
*
* Else store the converted value in @result, and return zero.
+ *
+ * This matches the behavior of strtol() on 32-bit platforms, even on
+ * platforms where long is 64-bits.
*/
int qemu_strtoi(const char *nptr, const char **endptr, int base,
int *result)
@@ -443,13 +446,15 @@
*
* Note that a number with a leading minus sign gets converted without
* the minus sign, checked for overflow (see above), then negated (in
- * @result's type). This is exactly how strtoul() works.
+ * @result's type). This matches the behavior of strtoul() on 32-bit
+ * platforms, even on platforms where long is 64-bits.
*/
int qemu_strtoui(const char *nptr, const char **endptr, int base,
unsigned int *result)
{
char *ep;
- long long lresult;
+ unsigned long long lresult;
+ bool neg;
assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
@@ -466,14 +471,22 @@
if (errno == ERANGE) {
*result = -1;
} else {
+ /*
+ * Note that platforms with 32-bit strtoul only accept input
+ * in the range [-4294967295, 4294967295]; but we used 64-bit
+ * strtoull which wraps -18446744073709551615 to 1 instead of
+ * declaring overflow. So we must check if '-' was parsed,
+ * and if so, undo the negation before doing our bounds check.
+ */
+ neg = memchr(nptr, '-', ep - nptr) != NULL;
+ if (neg) {
+ lresult = -lresult;
+ }
if (lresult > UINT_MAX) {
*result = UINT_MAX;
errno = ERANGE;
- } else if (lresult < INT_MIN) {
- *result = UINT_MAX;
- errno = ERANGE;
} else {
- *result = lresult;
+ *result = neg ? -lresult : lresult;
}
}
return check_strtox_error(nptr, ep, endptr, lresult == 0, errno);
