Re: [PATCH 1/1] btrfs: Simplify parsing max_inline mount option

[Anand Jain <anand.jain@xxxxxxxxxx>]

On 15/8/19 10:04 AM, Vladimir Panteleev wrote:
> - Avoid an allocation;
> - Properly handle non-numerical argument and garbage after numerical
>    argument.
>
> Signed-off-by: Vladimir Panteleev <git@xxxxxxxxxxxxxxxxxx>
> ---
>   fs/btrfs/super.c | 24 +++++++++---------------
>   1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index f56617dfb3cf..6fe8ef6667f3 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -426,7 +426,7 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
>   			unsigned long new_flags)
>   {
>   	substring_t args[MAX_OPT_ARGS];
> -	char *p, *num;
> +	char *p, *retptr;
>   	u64 cache_gen;
>   	int intarg;
>   	int ret = 0;
> @@ -630,22 +630,16 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
>   			info->thread_pool_size = intarg;
>   			break;
>   		case Opt_max_inline:
> -			num = match_strdup(&args[0]);
> -			if (num) {
> -				info->max_inline = memparse(num, NULL);
> -				kfree(num);
> -
> -				if (info->max_inline) {
> -					info->max_inline = min_t(u64,
> -						info->max_inline,
> -						info->sectorsize);
> -				}
> -				btrfs_info(info, "max_inline at %llu",
> -					   info->max_inline);
> -			} else {
> -				ret = -ENOMEM;
> +			info->max_inline = memparse(args[0].from, &retptr);


> +			if (retptr != args[0].to || info->max_inline == 0) {
> +				ret = -EINVAL;
>   				goto out;

 This causes regression, max_inline = 0 is a valid parameter.

Thanks, Anand


>   			}
> +			info->max_inline = min_t(u64,
> +				info->max_inline,
> +				info->sectorsize);
> +			btrfs_info(info, "max_inline at %llu",
> +				   info->max_inline);
>   			break;
>   		case Opt_alloc_start:
>   			btrfs_info(info,