Re: [PATCH] scsi: sd: Use sysfs_match_string()

[Bart Van Assche <Bart.VanAssche@sandisk.com>]

On Fri, 2017-05-26 at 13:02 -0400, Martin K. Petersen wrote:
> Avoid unnecessary snprintf() when formatting variables for display in
> sysfs and switch to sysfs_match_string() for validating user input.

Hello Martin,

Would it be worth it to split this patch into two patches - one for the
snprintf() conversion and another patch for the sysfs_match_string()
conversion?

> @@ -155,7 +155,7 @@ static ssize_t
>  cache_type_store(struct device *dev, struct device_attribute *attr,
>  		 const char *buf, size_t count)
>  {
> -	int i, ct = -1, rcd, wce, sp;
> +	int ct = -1, rcd, wce, sp;

Is it still necessary to initialize ct in this function?

> +	mode = sysfs_match_string(lbp_mode, buf);
> +	if (mode < 0)
>  		return -EINVAL;
> [ ... ]
> +	mode = sysfs_match_string(zeroing_mode, buf);
> +	if (mode < 0)
>  		return -EINVAL; 

sysfs_match_string() only supports dense arrays. Maybe it's worth to add a
comment above lbp_mode[] and zeroing_mode[] that these must be dense arrays
or that the above calls to sysfs_match_string() will break?

Otherwise this patch looks fine to me.

Bart.