Hi,
First of all, sorry for not reviewing this earlier. I thought
other people were already looking at the first 4 patches.
On Sun, Oct 20, 2019 at 07:11:14PM +0800, Tao Xu wrote:
To convert strings with time suffixes to numbers, support time unit are
"ps" for picosecond, "ns" for nanosecond, "us" for microsecond, "ms"
for millisecond or "s" for second.
Signed-off-by: Tao Xu <address@hidden>
---
No changes in v13.
---
include/qemu/cutils.h | 1 +
util/cutils.c | 82 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+)
diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index b54c847e0f..7b6d106bdd 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -182,5 +182,6 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n);
* *str1 is <, == or > than *str2.
*/
int qemu_pstrcmp0(const char **str1, const char **str2);
+int qemu_strtotime_ps(const char *nptr, const char **end, uint64_t *result);
#endif
diff --git a/util/cutils.c b/util/cutils.c
index fd591cadf0..a50c15f46a 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -847,3 +847,85 @@ int qemu_pstrcmp0(const char **str1, const char **str2)
{
return g_strcmp0(*str1, *str2);
}
+
+static int64_t timeunit_mul(const char *unitstr)
+{
+ if (g_strcmp0(unitstr, "ps") == 0) {
+ return 1;
This makes do_strtotime("123ps,...", &end, ...) fail because of
trailing data.
+ } else if (g_strcmp0(unitstr, "ns") == 0) {
+ return 1000;
+ } else if (g_strcmp0(unitstr, "us") == 0) {
+ return 1000000;
+ } else if (g_strcmp0(unitstr, "ms") == 0) {
+ return 1000000000LL;
+ } else if (g_strcmp0(unitstr, "s") == 0) {
+ return 1000000000000LL;
Same as above, for the other suffixes.
+ } else {
+ return -1;
But this makes do_strtotime("123,...", &end, ...) work as
expected.
This is inconsistent. I see that you are not testing non-NULL
`end` argument at test_qemu_strtotime_ps_trailing(), so that's
probably why this issue wasn't detected.
+ }
+}
+
+
+/*
+ * Convert string to time, support time unit are ps for picosecond,
+ * ns for nanosecond, us for microsecond, ms for millisecond or s for second.
+ * End pointer will be returned in *end, if not NULL. Return -ERANGE on
+ * overflow, and -EINVAL on other error.
+ */
+static int do_strtotime(const char *nptr, const char **end,
+ const char *default_unit, uint64_t *result)
+{
+ int retval;
+ const char *endptr;
+ int mul_required = 0;
+ int64_t mul;
+ double val, integral, fraction;
+
+ retval = qemu_strtod_finite(nptr, &endptr, &val);
+ if (retval) {
+ goto out;
+ }
+ fraction = modf(val, &integral);
+ if (fraction != 0) {
+ mul_required = 1;
+ }
+
+ mul = timeunit_mul(endptr);
+
+ if (mul == 1000000000000LL) {
+ endptr++;
+ } else if (mul != -1) {
+ endptr += 2;
This is fragile. It can break very easily if additional
one-letter suffixes are added to timeunit_mul() in the future.
One option to make this safer is to make timeunit_mul() get a
pointer to endptr.
+ } else {
+ mul = timeunit_mul(default_unit);
+ assert(mul >= 0);
+ }
+ if (mul == 1 && mul_required) {
+ retval = -EINVAL;
+ goto out;
+ }
+ /*
+ * Values >= 0xfffffffffffffc00 overflow uint64_t after their trip
+ * through double (53 bits of precision).
+ */
+ if ((val * (double)mul >= 0xfffffffffffffc00) || val < 0) {
+ retval = -ERANGE;
+ goto out;
+ }
+ *result = val * (double)mul;
+ retval = 0;
+
+out:
+ if (end) {
+ *end = endptr;
This indicates that having trailing data after the string is OK
if `end` is not NULL, but timeunit_mul() doesn't take that into
account.
Considering that this function is just a copy of do_strtosz(), I
suggest making do_strtosz() and suffix_mul() get a
suffix/multiplier table as input, instead of duplicating the
code.
Thanks for your suggestion, I will try it.