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

Qu Wenruo
2023-12-07T02:31:57Z

[-- Attachment #1.1.1: Type: text/plain, Size: 1581 bytes --]

On 2023/12/5 21:43, 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.

To me, I believe it's better to completely get rid of memparse().

As you already found out, some suffix, especially "e|E" can screw up the
result.
E.g. "25e" would be interpreted as 25 with "e" as suffix, which is fine
according to the rule. (without prefix, the base is 10, so only "25" is
valid. Then the remaining part is interpreted as suffix).

And since btrfs is not going to do pretty size output for sysfs (as most
sysfs is not directly for end users, and we need accurate output), to be
consistent there isn't much need for suffix handling either.

So can't we just replace memparse() with kstrtoull()?

Thanks,
Qu

>
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> ---
> fs/btrfs/sysfs.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index e6b51fb3ddc1e..75f3b774a4d83 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -1779,10 +1779,9 @@ static ssize_t btrfs_devinfo_scrub_speed_max_store(struct kobject *kobj,
> {
> struct btrfs_device *device = container_of(kobj, struct btrfs_device,
> devid_kobj);
> - char *endptr;
> unsigned long long limit;
>
> - limit = memparse(buf, &endptr);
> + limit = memparse(buf, NULL);
> WRITE_ONCE(device->scrub_speed_max, limit);
> return len;
> }

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7027 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]