Re: [PATCH v12 02/11] lib: kstrtox: add kstrtoudec64() and kstrtodec64()

[Rodrigo Alencar]

On 26/05/10 01:42PM, Rodrigo Alencar via B4 Relay wrote:
> From: Rodrigo Alencar <rodrigo.alencar@xxxxxxxxxx>
> 
> Add helpers that parses decimal numbers into 64-bit number, i.e., decimal
> point numbers with pre-defined scale are parsed into a 64-bit value (fixed
> precision). After the decimal point, digits beyond the specified scale
> are ignored.

Hi Andy,

I am starting over here, the other conversation is getting hard to follow.
This is my new proposal...

...

> +static int _kstrtoudec64(const char *s, unsigned int scale, u64 *res)
> +{
> +	u64 _res = 0, _frac = 0;
> +	unsigned int rv;
> +
> +	if (scale > 19) /* log10(2^64) = 19.26 */
> +		return -EINVAL;
> +
> +	if (*s != '.') {
> +		rv = _parse_integer(s, 10, &_res);
> +		if (rv & KSTRTOX_OVERFLOW)
> +			return -ERANGE;
> +		if (rv == 0)
> +			return -EINVAL;
> +		s += rv;
> +	}
> +
> +	if (*s == '.' && scale) {
> +		s++; /* skip decimal point */
> +		rv = _parse_integer_limit(s, 10, &_frac, scale);
> +		if (rv & KSTRTOX_OVERFLOW)
> +			return -ERANGE;
> +		if (rv == 0)
> +			return -EINVAL;
> +		s += rv;
> +		if (rv < scale)
> +			_frac *= int_pow(10, scale - rv);
> +		while (isdigit(*s)) /* truncate */
> +			s++;
> +	}
> +
> +	if (*s == '\n')
> +		s++;
> +	if (*s)
> +		return -EINVAL;
> +
> +	if (check_mul_overflow(_res, int_pow(10, scale), &_res) ||
> +	    check_add_overflow(_res, _frac, &_res))
> +		return -ERANGE;
> +
> +	*res = _res;
> +	return 0;
> +}

This function now becomes:

	static int _kstrtoudec64(const char *s, unsigned int scale, u64 *res)
	{
		u64 _res = 0;
		unsigned int rv_int, rv_frac;

		rv_int = _parse_integer(s, 10, &_res);
		if (rv_int & KSTRTOX_OVERFLOW)
			return -ERANGE;
		s += rv_int;

		if (*s == '.')
			s++; /* skip decimal point */

		rv_frac = _parse_integer_limit_init(s, 10, _res, &_res, scale);
		if (rv_frac & KSTRTOX_OVERFLOW)
			return -ERANGE;
		s += rv_frac;

		if (!rv_int && !rv_frac && !isdigit(*s))
			return -EINVAL; /* no digits at all */

		while (isdigit(*s)) /* truncate digits */
			s++;

		if (*s == '\n')
			s++;
		if (*s)
			return -EINVAL;

		if (_res && (scale > (19 + rv_frac) || /* log10(2^64) = 19.26 */
		    check_mul_overflow(_res, int_pow(10, scale - rv_frac), &_res)))
			return -ERANGE;

		*res = _res;
		return 0;
	}

The new thing here is _parse_integer_limit_init(), which is a local modified
helper that accepts an init value, so _parse_integer_limit() becomes:

	unsigned int _parse_integer_limit(const char *s, unsigned int base,
					  unsigned long long *p, size_t max_chars)
	{
		return _parse_integer_limit_init(s, base, 0, p, max_chars);
	}

with init = 0:

	static unsigned int _parse_integer_limit_init(const char *s, unsigned int base,
						      unsigned long long init,
						      unsigned long long *p,
						      size_t max_chars)
	{
		unsigned long long res;
		unsigned int rv;

		res = init;
		/* ...
		 * the rest is the same implementation as _parse_integer_limit()
		 * ...
		 */
		return rv;
	}

That allows to accumulate the final value into the same variable, which makes
things simpler and decreases the amount of overflow checks.

The scale can now be a bigger value, like 0.00000000000000000000000000000000423
can be parsed with scale = 35, resulting into 423.

The truncation loop is still there... I think this implementation is better,
and I am not sure what is the input limit that you would consider ok to allow
non-zero digits to be truncated once the scale can now be something bigger than 19.
As long as the output fits into a u64 variable, the parser still works.

I am also adding new test cases for that!

-- 
Kind regards,

Rodrigo Alencar