aboutsummaryrefslogtreecommitdiff
path: root/util
diff options
context:
space:
mode:
authorEric Blake <eblake@redhat.com>2023-05-22 14:04:37 -0500
committerEric Blake <eblake@redhat.com>2023-06-02 12:29:27 -0500
commit896fbd90aa3ffd4ad57b35722128fe65a9abc530 (patch)
tree6229e64084c501278acb16e1c2922dd8fbd2812b /util
parente1cf34b6b33caea5e47d46ba715eca6603fdbbc4 (diff)
downloadqemu-896fbd90aa3ffd4ad57b35722128fe65a9abc530.zip
qemu-896fbd90aa3ffd4ad57b35722128fe65a9abc530.tar.gz
qemu-896fbd90aa3ffd4ad57b35722128fe65a9abc530.tar.bz2
cutils: Set value in all qemu_strtosz* error paths
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> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-16-eblake@redhat.com>
Diffstat (limited to 'util')
-rw-r--r--util/cutils.c17
1 files changed, 12 insertions, 5 deletions
diff --git a/util/cutils.c b/util/cutils.c
index 1dc67d2..c5530a5 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;