Re: [PATCH] btrfs: drop unused memparse() parameter

[David Disseldorp <ddiss@suse.de>]

On Wed, 6 Dec 2023 19:53:31 +0100, David Sterba wrote:

> On Wed, Dec 06, 2023 at 11:21:43AM +1100, David Disseldorp wrote:
> > On Tue, 5 Dec 2023 15:22:53 +0100, David Sterba wrote:
> >   
> > > On Tue, Dec 05, 2023 at 10:13:29PM +1100, David Disseldorp wrote:  
> > > > The @retptr parameter for memparse() is optional.
> > > > btrfs_devinfo_scrub_speed_max_store() doesn't use it for any input
> > > > validation, so the parameter can be dropped.    
> > > 
> > > Or should it be used for validation? memparse is also used in
> > > btrfs_chunk_size_store() that accepts whitespace as trailing characters
> > > (namely '\n' if the value is from echo).  
> > 
> > It probably should have been used for validation when originally added,
> > but the current behaviour is now part of the sysfs scrub_speed_max API.
> > Failing on invalid input would break scripts which do things like
> >   echo clear > /sys/fs/btrfs/UUID/devinfo/1/scrub_speed_max  
> 
> I'm not sure the 'part of the API' is a valid agrument here. It's
> documented that the value is in bytes and that suffixes can be passed
> for convenience. How come anybody would use 'clear' in the first place
> and expect it to work with undefined meaning?

Most people don't read documentation :). If there's a willingness to
accept any fallout from adding the validation then I'm happy to do that.
Will send a v2.

Cheers, David