[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/1] utils: Use fixed-point arithmetic in qemu_strtosz
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v2 1/1] utils: Use fixed-point arithmetic in qemu_strtosz |
Date: |
Mon, 15 Mar 2021 17:17:48 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 |
On 3/15/21 4:58 PM, Richard Henderson wrote:
> Once we've parsed the fractional value, extract it into an integral
> 64-bit fraction. Perform the scaling with integer arithemetic, and
Typo "arithmetic"
> simplify the overflow detection.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> tests/unit/test-cutils.c | 2 +-
> util/cutils.c | 50 ++++++++++++++++++++++++++++------------
> 2 files changed, 36 insertions(+), 16 deletions(-)
>
> diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
> index bad3a60993..e025b54c05 100644
> --- a/tests/unit/test-cutils.c
> +++ b/tests/unit/test-cutils.c
> @@ -2128,7 +2128,7 @@ static void test_qemu_strtosz_float(void)
> str = "12.345M";
> err = qemu_strtosz(str, &endptr, &res);
> g_assert_cmpint(err, ==, 0);
> - g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB));
> + g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB + 0.5));
> g_assert(endptr == str + 7);
> }
>
> diff --git a/util/cutils.c b/util/cutils.c
> index d89a40a8c3..c442882b88 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -275,10 +275,9 @@ static int do_strtosz(const char *nptr, const char **end,
> int retval;
> const char *endptr, *f;
> unsigned char c;
> - bool mul_required = false, hex = false;
> - uint64_t val;
> + bool hex = false;
> + uint64_t val, valf = 0;
> int64_t mul;
> - double fraction = 0.0;
>
> /* Parse integral portion as decimal. */
> retval = qemu_strtou64(nptr, &endptr, 10, &val);
> @@ -308,17 +307,19 @@ static int do_strtosz(const char *nptr, const char
> **end,
> * without fractional digits. If we see an exponent, treat
> * the entire input as invalid instead.
> */
> + double fraction;
> +
> f = endptr;
> retval = qemu_strtod_finite(f, &endptr, &fraction);
> if (retval) {
> - fraction = 0.0;
> endptr++;
> } else if (memchr(f, 'e', endptr - f) || memchr(f, 'E', endptr - f))
> {
> endptr = nptr;
> retval = -EINVAL;
> goto out;
> - } else if (fraction != 0) {
> - mul_required = true;
I'm glad you removed this float-equal warning:
util/cutils.c: In function ‘do_strtosz’:
util/cutils.c:320:29: error: comparing floating-point with ‘==’ or ‘!=’
is unsafe [-Werror=float-equal]
320 | } else if (fraction != 0) {
| ^~
cc1: all warnings being treated as errors
> + } else {
> + /* Extract into a 64-bit fixed-point fraction. */
> + valf = (uint64_t)(fraction * 0x1p64);
> }
> }
> c = *endptr;
> @@ -333,16 +334,35 @@ static int do_strtosz(const char *nptr, const char
> **end,
> mul = suffix_mul(default_suffix, unit);
> assert(mul > 0);
> }
> - if (mul == 1 && mul_required) {
> - endptr = nptr;
> - retval = -EINVAL;
> - goto out;
> + if (mul == 1) {
> + /* When a fraction is present, a scale is required. */
> + if (valf != 0) {
> + endptr = nptr;
> + retval = -EINVAL;
> + goto out;
> + }
> + } else {
> + uint64_t valh, tmp;
> +
> + /* Compute exact result: 64.64 x 64.0 -> 128.64 fixed point */
> + mulu64(&val, &valh, val, mul);
> + mulu64(&valf, &tmp, valf, mul);
> + val += tmp;
> + valh += val < tmp;
> +
> + /* Round 0.5 upward. */
> + tmp = valf >> 63;
> + val += tmp;
> + valh += val < tmp;
> +
> + /* Report overflow. */
> + if (valh != 0) {
> + retval = -ERANGE;
> + goto out;
> + }
> }
> - if (val > (UINT64_MAX - ((uint64_t) (fraction * mul))) / mul) {
> - retval = -ERANGE;
> - goto out;
> - }
> - *result = val * mul + (uint64_t) (fraction * mul);
> +
> + *result = val;
> retval = 0;
>
> out:
>