On 12.05.23 04:10, Eric Blake wrote:
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. Fix those, and update
the testsuite now that our bounds checks are correct.
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.
Fixes: 473a2a331e ("cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned
int types", v2.12.0)
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
tests/unit/test-cutils.c | 11 +++++------
util/cutils.c | 14 ++++++++++----
2 files changed, 15 insertions(+), 10 deletions(-)
Reviewed-by: Hanna Czenczek <hre...@redhat.com>
diff --git a/util/cutils.c b/util/cutils.c
index 5887e744140..997ddcd09e5 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -466,10 +466,16 @@ int qemu_strtoui(const char *nptr, const char **endptr,
int base,
if (errno == ERANGE) {
*result = -1;
} else {
- if (lresult > UINT_MAX) {
- *result = UINT_MAX;
- errno = ERANGE;
- } else if (lresult < INT_MIN) {
+ /*
+ * Note that platforms with 32-bit strtoul accept input in the
+ * range [-4294967295, 4294967295]; but we used 64-bit
+ * strtoull which wraps -18446744073709551615 to 1. Reject
+ * positive values that contain '-', and wrap all valid
+ * negative values.
+ */
+ if (lresult > UINT_MAX ||
+ lresult < -(long long)UINT_MAX ||
+ (lresult > 0 && memchr(nptr, '-', ep - nptr))) {
*result = UINT_MAX;
errno = ERANGE;
} else {
Just a question whether I guessed correctly, because there’s no comment
on the matter: We store the (supposedly unsigned) result of strtoull()
in a signed long long because e.g. -1 is mapped to ULLONG_MAX, so the
valid unsigned ranges would be [0, UINT_MAX] \cup [ULLONG_MAX - UINT_MAX
+ 1, ULLONG_MAX], which is more cumbersome to check than the [-UINT_MAX,
UINT_MAX] range? (And we’d need to exclude strings with - in them if
ullresult > UINT_MAX rather than > 0, probably)
Hanna
