* [f2fs-dev] [PATCH] f2fs: avoid to create an empty string as the extension_list
@ 2021-07-09 2:05 Wang Xiaojun
2021-07-09 3:21 ` Chao Yu
0 siblings, 1 reply; 5+ messages in thread
From: Wang Xiaojun @ 2021-07-09 2:05 UTC (permalink / raw
To: chao, jaegeuk; +Cc: linux-f2fs-devel
When creating a file, we need to set the temperature based on
extension_list. If the empty string is a valid extension_list,
the is_extension_exist will always returns true,
which affects the separation of hot and cold.
Signed-off-by: Wang Xiaojun <wangxiaojun11@huawei.com>
---
fs/f2fs/namei.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index a9cd9cf97229..34341d3edb8d 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -219,6 +219,8 @@ int f2fs_update_extension_list(struct f2fs_sb_info *sbi, const char *name,
int start, count;
int i;
+ if (!strlen(name))
+ return -EINVAL;
if (set) {
if (total_count == F2FS_MAX_EXTENSION)
return -EINVAL;
--
2.25.4
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: avoid to create an empty string as the extension_list
2021-07-09 2:05 [f2fs-dev] [PATCH] f2fs: avoid to create an empty string as the extension_list Wang Xiaojun
@ 2021-07-09 3:21 ` Chao Yu
2021-07-09 3:49 ` wangxiaojun (N)
0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2021-07-09 3:21 UTC (permalink / raw
To: Wang Xiaojun, jaegeuk; +Cc: linux-f2fs-devel
On 2021/7/9 10:05, Wang Xiaojun wrote:
> When creating a file, we need to set the temperature based on
> extension_list. If the empty string is a valid extension_list,
> the is_extension_exist will always returns true,
> which affects the separation of hot and cold.
>
> Signed-off-by: Wang Xiaojun <wangxiaojun11@huawei.com>
> ---
> fs/f2fs/namei.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index a9cd9cf97229..34341d3edb8d 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -219,6 +219,8 @@ int f2fs_update_extension_list(struct f2fs_sb_info *sbi, const char *name,
> int start, count;
> int i;
>
> + if (!strlen(name))
> + return -EINVAL;
How about adding this in __sbi_store()? like:
if (!strlen(name) || strlen(name) >= F2FS_EXTENSION_LEN)
return -EINVAL;
Otherwise, it looks good to me.
Reviewed-by: Chao Yu <chao@kernel.org>
Thanks,
> if (set) {
> if (total_count == F2FS_MAX_EXTENSION)
> return -EINVAL;
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: avoid to create an empty string as the extension_list
2021-07-09 3:21 ` Chao Yu
@ 2021-07-09 3:49 ` wangxiaojun (N)
2021-07-09 4:02 ` Chao Yu
0 siblings, 1 reply; 5+ messages in thread
From: wangxiaojun (N) @ 2021-07-09 3:49 UTC (permalink / raw
To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel
在 2021/7/9 11:21, Chao Yu 写道:
> On 2021/7/9 10:05, Wang Xiaojun wrote:
>> When creating a file, we need to set the temperature based on
>> extension_list. If the empty string is a valid extension_list,
>> the is_extension_exist will always returns true,
>> which affects the separation of hot and cold.
>>
>> Signed-off-by: Wang Xiaojun <wangxiaojun11@huawei.com>
>> ---
>> fs/f2fs/namei.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
>> index a9cd9cf97229..34341d3edb8d 100644
>> --- a/fs/f2fs/namei.c
>> +++ b/fs/f2fs/namei.c
>> @@ -219,6 +219,8 @@ int f2fs_update_extension_list(struct
>> f2fs_sb_info *sbi, const char *name,
>> int start, count;
>> int i;
>> + if (!strlen(name))
>> + return -EINVAL;
>
> How about adding this in __sbi_store()? like:
>
> if (!strlen(name) || strlen(name) >= F2FS_EXTENSION_LEN)
> return -EINVAL;
>
> Otherwise, it looks good to me.
This is an alternative modification. Exception check is more compact here.
But if the f2fs_update_extension_list function is called elsewhere in
the future,
checking inside the function can avoid this problem.
I was also a little unsure, and finally chose this modification.
>
> Reviewed-by: Chao Yu <chao@kernel.org>
>
> Thanks,
>
>> if (set) {
>> if (total_count == F2FS_MAX_EXTENSION)
>> return -EINVAL;
>>
> .
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: avoid to create an empty string as the extension_list
2021-07-09 3:49 ` wangxiaojun (N)
@ 2021-07-09 4:02 ` Chao Yu
2021-07-09 4:39 ` wangxiaojun (N)
0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2021-07-09 4:02 UTC (permalink / raw
To: wangxiaojun (N), jaegeuk; +Cc: linux-f2fs-devel
On 2021/7/9 11:49, wangxiaojun (N) wrote:
>
> 在 2021/7/9 11:21, Chao Yu 写道:
>> On 2021/7/9 10:05, Wang Xiaojun wrote:
>>> When creating a file, we need to set the temperature based on
>>> extension_list. If the empty string is a valid extension_list,
>>> the is_extension_exist will always returns true,
>>> which affects the separation of hot and cold.
>>>
>>> Signed-off-by: Wang Xiaojun <wangxiaojun11@huawei.com>
>>> ---
>>> fs/f2fs/namei.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
>>> index a9cd9cf97229..34341d3edb8d 100644
>>> --- a/fs/f2fs/namei.c
>>> +++ b/fs/f2fs/namei.c
>>> @@ -219,6 +219,8 @@ int f2fs_update_extension_list(struct
>>> f2fs_sb_info *sbi, const char *name,
>>> int start, count;
>>> int i;
>>> + if (!strlen(name))
>>> + return -EINVAL;
>>
>> How about adding this in __sbi_store()? like:
>>
>> if (!strlen(name) || strlen(name) >= F2FS_EXTENSION_LEN)
>> return -EINVAL;
>>
>> Otherwise, it looks good to me.
>
> This is an alternative modification. Exception check is more compact here.
>
> But if the f2fs_update_extension_list function is called elsewhere in
> the future,
There is other related check of @name in __sbi_store(), so I guess
f2fs_update_extension_list() is coupling to __sbi_store() now.
>
> checking inside the function can avoid this problem.
>
> I was also a little unsure, and finally chose this modification.
I'm fine to move all checks into f2fs_update_extension_list(), but now
it looks more clean to just let the caller do the check on @name.
Thanks,
>
>>
>> Reviewed-by: Chao Yu <chao@kernel.org>
>>
>> Thanks,
>>
>>> if (set) {
>>> if (total_count == F2FS_MAX_EXTENSION)
>>> return -EINVAL;
>>>
>> .
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: avoid to create an empty string as the extension_list
2021-07-09 4:02 ` Chao Yu
@ 2021-07-09 4:39 ` wangxiaojun (N)
0 siblings, 0 replies; 5+ messages in thread
From: wangxiaojun (N) @ 2021-07-09 4:39 UTC (permalink / raw
To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel
在 2021/7/9 12:02, Chao Yu 写道:
> On 2021/7/9 11:49, wangxiaojun (N) wrote:
>>
>> 在 2021/7/9 11:21, Chao Yu 写道:
>>> On 2021/7/9 10:05, Wang Xiaojun wrote:
>>>> When creating a file, we need to set the temperature based on
>>>> extension_list. If the empty string is a valid extension_list,
>>>> the is_extension_exist will always returns true,
>>>> which affects the separation of hot and cold.
>>>>
>>>> Signed-off-by: Wang Xiaojun <wangxiaojun11@huawei.com>
>>>> ---
>>>> fs/f2fs/namei.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
>>>> index a9cd9cf97229..34341d3edb8d 100644
>>>> --- a/fs/f2fs/namei.c
>>>> +++ b/fs/f2fs/namei.c
>>>> @@ -219,6 +219,8 @@ int f2fs_update_extension_list(struct
>>>> f2fs_sb_info *sbi, const char *name,
>>>> int start, count;
>>>> int i;
>>>> + if (!strlen(name))
>>>> + return -EINVAL;
>>>
>>> How about adding this in __sbi_store()? like:
>>>
>>> if (!strlen(name) || strlen(name) >= F2FS_EXTENSION_LEN)
>>> return -EINVAL;
>>>
>>> Otherwise, it looks good to me.
>>
>> This is an alternative modification. Exception check is more compact
>> here.
>>
>> But if the f2fs_update_extension_list function is called elsewhere in
>> the future,
>
> There is other related check of @name in __sbi_store(), so I guess
> f2fs_update_extension_list() is coupling to __sbi_store() now.
>
>>
>> checking inside the function can avoid this problem.
>>
>> I was also a little unsure, and finally chose this modification.
>
> I'm fine to move all checks into f2fs_update_extension_list(), but now
> it looks more clean to just let the caller do the check on @name.
>
> Thanks,
>
Okay, I'll update in the v2 version as you suggest. It's definitely more
clean.
Thanks,
>>
>>>
>>> Reviewed-by: Chao Yu <chao@kernel.org>
>>>
>>> Thanks,
>>>
>>>> if (set) {
>>>> if (total_count == F2FS_MAX_EXTENSION)
>>>> return -EINVAL;
>>>>
>>> .
> .
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-09 4:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-09 2:05 [f2fs-dev] [PATCH] f2fs: avoid to create an empty string as the extension_list Wang Xiaojun
2021-07-09 3:21 ` Chao Yu
2021-07-09 3:49 ` wangxiaojun (N)
2021-07-09 4:02 ` Chao Yu
2021-07-09 4:39 ` wangxiaojun (N)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.