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

[David Sterba <dsterba@suse.cz>]

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?