Re: [PATCH v2] usb-storage: Optimize scan delay more precisely

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux