aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarkus Armbruster <armbru@redhat.com>2011-11-22 09:46:06 +0100
committerAnthony Liguori <aliguori@us.ibm.com>2011-11-28 16:20:52 -0600
commiteba90e4efc80bc30c7d952ee6ea442207517a0da (patch)
tree427c6057aebd810361c0f2bbcc0339f47524b713
parente36b369577784ac59a7d74725368eec66f23ad52 (diff)
downloadqemu-arm-eba90e4efc80bc30c7d952ee6ea442207517a0da.tar.gz
cutils: Make strtosz & friends leave follow set to callers
strtosz() & friends require the size to be at the end of the string, or be followed by whitespace or ','. I find this surprising, because the name suggests it works like strtol(). The check simplifies callers that accept exactly that follow set slightly. No such callers exist. The check is redundant for callers that accept a smaller follow set, and thus need to check themselves anyway. Right now, this is the case for all but one caller. All of them neglected to check, or checked incorrectly, but the previous few commits fixed them up. Finally, the check is problematic for callers that accept a larger follow set. This is the case in monitor_parse_command(). Fortunately, the problems there are relatively harmless. monitor_parse_command() uses strtosz() for argument type 'o'. When the last argument is of type 'o', a trailing ',' is diagnosed differently than other trailing junk: (qemu) migrate_set_speed 1x invalid size (qemu) migrate_set_speed 1, migrate_set_speed: extraneous characters at the end of line A related inconsistency exists with non-last arguments. No such command exists, but let's use memsave to explore the inconsistency. The monitor permits, but does not require whitespace between arguments. For instance, "memsave (1-1)1024foo" is parsed as command memsave with three arguments 0, 1024 and "foo". Yes, this is daft, but at least it's consistently daft. If I change memsave's second argument from 'i' to 'o', then "memsave (1-1)1foo" is rejected, because the size is followed by an 'f'. But "memsave (1-1)1," is still accepted, and duly saves to file ",". We don't have any users of strtosz that profit from the check. In the users we have, it appears to encourage sloppy error checking, or gets in the way. Drop the bothersome check. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
-rw-r--r--cutils.c70
1 files changed, 26 insertions, 44 deletions
diff --git a/cutils.c b/cutils.c
index 55b059645b..6db6304bb4 100644
--- a/cutils.c
+++ b/cutils.c
@@ -315,18 +315,34 @@ int fcntl_setfl(int fd, int flag)
}
#endif
+static int64_t suffix_mul(char suffix, int64_t unit)
+{
+ switch (qemu_toupper(suffix)) {
+ case STRTOSZ_DEFSUFFIX_B:
+ return 1;
+ case STRTOSZ_DEFSUFFIX_KB:
+ return unit;
+ case STRTOSZ_DEFSUFFIX_MB:
+ return unit * unit;
+ case STRTOSZ_DEFSUFFIX_GB:
+ return unit * unit * unit;
+ case STRTOSZ_DEFSUFFIX_TB:
+ return unit * unit * unit * unit;
+ }
+ return -1;
+}
+
/*
* Convert string to bytes, allowing either B/b for bytes, K/k for KB,
* M/m for MB, G/g for GB or T/t for TB. End pointer will be returned
- * in *end, if not NULL. A valid value must be terminated by
- * whitespace, ',' or '\0'. Return -1 on error.
+ * in *end, if not NULL. Return -1 on error.
*/
int64_t strtosz_suffix_unit(const char *nptr, char **end,
const char default_suffix, int64_t unit)
{
int64_t retval = -1;
char *endptr;
- unsigned char c, d;
+ unsigned char c;
int mul_required = 0;
double val, mul, integral, fraction;
@@ -339,51 +355,17 @@ int64_t strtosz_suffix_unit(const char *nptr, char **end,
if (fraction != 0) {
mul_required = 1;
}
- /*
- * Any whitespace character is fine for terminating the number,
- * in addition we accept ',' to handle strings where the size is
- * part of a multi token argument.
- */
c = *endptr;
- d = c;
- if (qemu_isspace(c) || c == '\0' || c == ',') {
- c = 0;
- d = default_suffix;
+ mul = suffix_mul(c, unit);
+ if (mul >= 0) {
+ endptr++;
+ } else {
+ mul = suffix_mul(default_suffix, unit);
+ assert(mul >= 0);
}
- switch (qemu_toupper(d)) {
- case STRTOSZ_DEFSUFFIX_B:
- mul = 1;
- if (mul_required) {
- goto fail;
- }
- break;
- case STRTOSZ_DEFSUFFIX_KB:
- mul = unit;
- break;
- case STRTOSZ_DEFSUFFIX_MB:
- mul = unit * unit;
- break;
- case STRTOSZ_DEFSUFFIX_GB:
- mul = unit * unit * unit;
- break;
- case STRTOSZ_DEFSUFFIX_TB:
- mul = unit * unit * unit * unit;
- break;
- default:
+ if (mul == 1 && mul_required) {
goto fail;
}
- /*
- * If not terminated by whitespace, ',', or \0, increment endptr
- * to point to next character, then check that we are terminated
- * by an appropriate separating character, ie. whitespace, ',', or
- * \0. If not, we are seeing trailing garbage, thus fail.
- */
- if (c != 0) {
- endptr++;
- if (!qemu_isspace(*endptr) && *endptr != ',' && *endptr != 0) {
- goto fail;
- }
- }
if ((val * mul >= INT64_MAX) || val < 0) {
goto fail;
}