On Fri, Apr 05, 2024 at 11:17:38AM +0900, Norihiko Hama wrote:
> +static int delay_use_set(const char *s, const struct kernel_param *kp)
> +{
> + unsigned long long delay_ms = 0;
> + int scale = MSEC_PER_SEC;
> + const char *p = skip_spaces(s);
> +
> + if (!isdigit(*p))
> + return -EINVAL;
> +
> + while (isdigit(*p)) {
> + delay_ms *= 10;
> + delay_ms += scale * (*p++ - '0');
> + if (delay_ms != (unsigned int)delay_ms)
> + return -ERANGE;
> + }
> +
> + if (*p == '.' && isdigit(*(p + 1))) {
> + p++;
> + while (isdigit(*p)) {
> + scale /= 10;
> + if (scale == 0)
> + return -EINVAL;
> + delay_ms += scale * (*p++ - '0');
> + if (delay_ms != (unsigned int)delay_ms)
> + return -ERANGE;
> + }
> + }
> + if (*p == '\n')
> + p++;
> + if (*p)
> + return -EINVAL;
> +
> + *((unsigned int *)kp->arg) = delay_ms;
> + return 0;
> +}
I don't think rolling your own routine like this for parsing fixed-point
values is a very good idea. On the other hand, I can't find any other
code in the kernel for doing it, so it may be unavoidable.
Regardless, this doesn't seem like the best approach. The parsing
should be done in a separate routine, and it doesn't have to do all the
work by itself. For example, it could copy the input data to a
temporary buffer, leaving out the decimal point and padding with zeros
on the right so that there are always exactly three fractional digits.
Then it could call a kstrtouint() to do the rest of the work.
Alan Stern
