Received-SPF: pass (google.com: domain of linux-kernel+bounces-11903-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <7d2bb5eb-a9cf-4884-aa75-ba9d6af9767f@gmx.com>
Date: Wed, 27 Dec 2023 18:56:57 +1030
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 3/3] btrfs: migrate to the newer memparse_safe() helper
To: David Disseldorp <ddiss@suse.de>, Qu Wenruo <wqu@suse.com>
Cc: linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
 akpm@linux-foundation.org, christophe.jaillet@wanadoo.fr,
 andriy.shevchenko@linux.intel.com, David.Laight@ACULAB.COM
References: <cover.1703324146.git.wqu@suse.com>
 <6dfa53ded887caa2269c1beeaedcff086342339a.1703324146.git.wqu@suse.com>
 <20231227172709.4402bc6c@echidna>
Content-Language: en-US
From: Qu Wenruo <quwenruo.btrfs@gmx.com>
Autocrypt: addr=quwenruo.btrfs@gmx.com; keydata=
 xsBNBFnVga8BCACyhFP3ExcTIuB73jDIBA/vSoYcTyysFQzPvez64TUSCv1SgXEByR7fju3o
 8RfaWuHCnkkea5luuTZMqfgTXrun2dqNVYDNOV6RIVrc4YuG20yhC1epnV55fJCThqij0MRL
 1NxPKXIlEdHvN0Kov3CtWA+R1iNN0RCeVun7rmOrrjBK573aWC5sgP7YsBOLK79H3tmUtz6b
 9Imuj0ZyEsa76Xg9PX9Hn2myKj1hfWGS+5og9Va4hrwQC8ipjXik6NKR5GDV+hOZkktU81G5
 gkQtGB9jOAYRs86QG/b7PtIlbd3+pppT0gaS+wvwMs8cuNG+Pu6KO1oC4jgdseFLu7NpABEB
 AAHNIlF1IFdlbnJ1byA8cXV3ZW5ydW8uYnRyZnNAZ214LmNvbT7CwJQEEwEIAD4CGwMFCwkI
 BwIGFQgJCgsCBBYCAwECHgECF4AWIQQt33LlpaVbqJ2qQuHCPZHzoSX+qAUCY00iVQUJDToH
 pgAKCRDCPZHzoSX+qNKACACkjDLzCvcFuDlgqCiS4ajHAo6twGra3uGgY2klo3S4JespWifr
 BLPPak74oOShqNZ8yWzB1Bkz1u93Ifx3c3H0r2vLWrImoP5eQdymVqMWmDAq+sV1Koyt8gXQ
 XPD2jQCrfR9nUuV1F3Z4Lgo+6I5LjuXBVEayFdz/VYK63+YLEAlSowCF72Lkz06TmaI0XMyj
 jgRNGM2MRgfxbprCcsgUypaDfmhY2nrhIzPUICURfp9t/65+/PLlV4nYs+DtSwPyNjkPX72+
 LdyIdY+BqS8cZbPG5spCyJIlZonADojLDYQq4QnufARU51zyVjzTXMg5gAttDZwTH+8LbNI4
 mm2YzsBNBFnVga8BCACqU+th4Esy/c8BnvliFAjAfpzhI1wH76FD1MJPmAhA3DnX5JDORcga
 CbPEwhLj1xlwTgpeT+QfDmGJ5B5BlrrQFZVE1fChEjiJvyiSAO4yQPkrPVYTI7Xj34FnscPj
 /IrRUUka68MlHxPtFnAHr25VIuOS41lmYKYNwPNLRz9Ik6DmeTG3WJO2BQRNvXA0pXrJH1fN
 GSsRb+pKEKHKtL1803x71zQxCwLh+zLP1iXHVM5j8gX9zqupigQR/Cel2XPS44zWcDW8r7B0
 q1eW4Jrv0x19p4P923voqn+joIAostyNTUjCeSrUdKth9jcdlam9X2DziA/DHDFfS5eq4fEv
 ABEBAAHCwHwEGAEIACYCGwwWIQQt33LlpaVbqJ2qQuHCPZHzoSX+qAUCY00ibgUJDToHvwAK
 CRDCPZHzoSX+qK6vB/9yyZlsS+ijtsvwYDjGA2WhVhN07Xa5SBBvGCAycyGGzSMkOJcOtUUf
 tD+ADyrLbLuVSfRN1ke738UojphwkSFj4t9scG5A+U8GgOZtrlYOsY2+cG3R5vjoXUgXMP37
 INfWh0KbJodf0G48xouesn08cbfUdlphSMXujCA8y5TcNyRuNv2q5Nizl8sKhUZzh4BascoK
 DChBuznBsucCTAGrwPgG4/ul6HnWE8DipMKvkV9ob1xJS2W4WJRPp6QdVrBWJ9cCdtpR6GbL
 iQi22uZXoSPv/0oUrGU+U5X4IvdnvT+8viPzszL5wXswJZfqfy8tmHM85yjObVdIG6AlnrrD
In-Reply-To: <20231227172709.4402bc6c@echidna>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:MZERzLhhOQo=;CIRSspjnIJHDL757gvXdbqkB/BJ
 IWqsdjGl1Le+9D6dqhDxu0DrN+GpZ/ie9S3pw5qmz7sEZnTTuMzkJmdPt1VFJsewKDBtv01rp
 6b3gB85FlDxTFfiaQBjynCQnMw1vh1DNhuk2NAD8/z5i6ANVOGGtF6ybGIv7gRICRtXCa5ATT
 m9+MNM6QTx6ycj6VsjwgJrIwAbRSqTQdt+V/6N/NwOwR/kv9HJ+KL28x7t5c6fYqO9QDp4SHk
 hm7kpSZ+azbYk1txOLTSMcjVctpg++FeekdKLyum4T3bvd0oPBGUfmw1hOhSfz09FRBdKLA6S
 vUBR9rhswy7qG7WJsfJIAedYsphHP/paictmZGpjB32nd31OyVvg+ZbVHLAvErN9ZKb/54t6S
 OE2NK1VvWw9w/9t7/J2/W8gydfky8b6nwj7wtApxBRPUWV/t1lME4BK3o1F1VHtUP5MudfU1w
 QbskJoypb6RvY4Gq4LW2MNouIe/fJim0pnfkrefgqBEHfKhtmw5Dtt4iCoCjLFa/YQHqTxBhv
 EmoTdpFf5BXgcRjKN/mWO7jKb2BPZYt+2HMWCjsiN/KvGNM8WfAiZoPnJ26y5p2SF/jia0zgT
 Obapdr1r++mo+6CULYwhZjxSfwDzwpT2dw2KxeODp23ni07bDJVSkNHB/QZbnF9j7fWBpKl/x
 2qqx2/7cAjP7T6zpwncl8Eyap7RK2+K+mFEFXzPQkwftNbPYXQFktcko5+EDWo471wnmN6ZxD
 Zz/HgBgLfffxb1cU/R4nzwVS7HHRhlYvz3XqiOeQRUSqAPmUI+KqjNzI/s0uzgBWOXJ4jIvgS
 aUAhipzJShBMza3xGNvVIfnt1+8b4mCLksXvQk7s2MmrWQ5fYsURDSLdGghEabNn3vWQ8Gu2o
 AR4nBBPbmEzVEDAh1lHXxtmsmuh/5B24BhSNADBJy1Iv5Ul3fOfWyRN6icrl2/FmSsVJESgtc
 E7XfEUpq2uMJaa2tazJyVaOhvqM=


On 2023/12/27 16:57, David Disseldorp wrote:
> On Sat, 23 Dec 2023 20:28:07 +1030, Qu Wenruo wrote:
>
>> The new helper has better error report and correct overflow detection,
>> furthermore the old @retptr behavior is also kept, thus there should be
>> no behavior change.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   fs/btrfs/ioctl.c |  8 ++++++--
>>   fs/btrfs/super.c |  8 ++++++++
>>   fs/btrfs/sysfs.c | 14 +++++++++++---
>>   3 files changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 4e50b62db2a8..8bfd4b4ccf02 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -1175,8 +1175,12 @@ static noinline int btrfs_ioctl_resize(struct fi=
le *file,
>>   			mod =3D 1;
>>   			sizestr++;
>>   		}
>> -		new_size =3D memparse(sizestr, &retptr);
>> -		if (*retptr !=3D '\0' || new_size =3D=3D 0) {
>> +
>> +		ret =3D memparse_safe(sizestr, MEMPARSE_SUFFIXES_DEFAULT,
>> +				    &new_size, &retptr);
>> +		if (ret < 0)
>> +			goto out_finish;
>> +		if (*retptr !=3D '\0') {
>
> Was dropping the -EINVAL return for new_size=3D0 intentional?

Oh, that's unintentional. Although we would reject the invalid string, a
dedicated "0" can still be parsed.
In that case we should still return -EINVAL.

I just got it confused with the old behavior for invalid string (where 0
is returned and @retptr is not advanced).
>
>>   			ret =3D -EINVAL;
>>   			goto out_finish;
>>   		}
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 3a677b808f0f..2bb6ea525e89 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -400,6 +400,14 @@ static int btrfs_parse_param(struct fs_context *fc=
, struct fs_parameter *param)
>>   		ctx->thread_pool_size =3D result.uint_32;
>>   		break;
>>   	case Opt_max_inline:
>> +		int ret;
>> +
>> +		ret =3D memparse_safe(param->string, MEMPARSE_SUFFIXES_DEFAULT,
>> +				    &ctx->max_inline, NULL);
>> +		if (ret < 0) {
>> +			btrfs_err(NULL, "invalid string \"%s\"", param->string);
>> +			return ret;
>> +		}
>>   		ctx->max_inline =3D memparse(param->string, NULL);
>
> Looks like you overlooked removal of the old memparse() call above.

My bad, I forgot to remove the old line.

Furthermore, the declaration of "ret" inside case block is not allowed,
I'll fix it anyway.

Thanks,
Qu
>
> Cheers, David
>