Re: [PATCH] f2fs: Replace strncpy with memcpy
On 2018/7/2 10:16, Guenter Roeck wrote:
> On 07/01/2018 06:53 PM, Chao Yu wrote:
>> On 2018/7/2 4:57, Guenter Roeck wrote:
>>> gcc 8.1.0 complains:
>>>
>>> fs/f2fs/namei.c: In function 'f2fs_update_extension_list':
>>> fs/f2fs/namei.c:257:3: warning:
>>> 'strncpy' output truncated before terminating nul copying
>>> as many bytes from a string as its length
>>> fs/f2fs/namei.c:249:3: warning:
>>> 'strncpy' output truncated before terminating nul copying
>>> as many bytes from a string as its length
>>>
>>> Using strncpy() is indeed less than perfect since the length of data to
>>> be copied has already been determined with strlen(). Replace strncpy()
>>> with memcpy() to address the warning and optimize the code a little.
>>>
>>> Signed-off-by: Guenter Roeck
>>> ---
>>> fs/f2fs/namei.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
>>> index 231b7f3ea7d3..e75607544f7c 100644
>>> --- a/fs/f2fs/namei.c
>>> +++ b/fs/f2fs/namei.c
>>> @@ -246,7 +246,7 @@ int f2fs_update_extension_list(struct f2fs_sb_info
>>> *sbi, const char *name,
>>> return -EINVAL;
>>>
>>> if (hot) {
>>> - strncpy(extlist[count], name, strlen(name));
>>> + memcpy(extlist[count], name, strlen(name));
>>
>> How about replacing with strcpy(extlist[count], name)? Because name length
>> has
>> already been checked before f2fs_update_extension_list, it should be valid,
>> and
>> will not cause overflow during copying.
>>
>
> Your call; feel free to submit an alternative. Since it is different files,
> static
> analysis might not know and complain, though. You might want to make sure
> that this
> doesn't happen, and also add a comment explaining the reason for using
> strcpy().
Yeah, that could be changed in another patch, but it will be trivial. Anyway, to
fix this gcc complaint, this patch looks good to me, thanks for the patch. :)
Reviewed-by: Chao Yu
Thanks,
>
> Thanks,
> Guenter
>
>
Re: [PATCH] f2fs: Replace strncpy with memcpy
On 2018/7/2 10:16, Guenter Roeck wrote:
> On 07/01/2018 06:53 PM, Chao Yu wrote:
>> On 2018/7/2 4:57, Guenter Roeck wrote:
>>> gcc 8.1.0 complains:
>>>
>>> fs/f2fs/namei.c: In function 'f2fs_update_extension_list':
>>> fs/f2fs/namei.c:257:3: warning:
>>> 'strncpy' output truncated before terminating nul copying
>>> as many bytes from a string as its length
>>> fs/f2fs/namei.c:249:3: warning:
>>> 'strncpy' output truncated before terminating nul copying
>>> as many bytes from a string as its length
>>>
>>> Using strncpy() is indeed less than perfect since the length of data to
>>> be copied has already been determined with strlen(). Replace strncpy()
>>> with memcpy() to address the warning and optimize the code a little.
>>>
>>> Signed-off-by: Guenter Roeck
>>> ---
>>> fs/f2fs/namei.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
>>> index 231b7f3ea7d3..e75607544f7c 100644
>>> --- a/fs/f2fs/namei.c
>>> +++ b/fs/f2fs/namei.c
>>> @@ -246,7 +246,7 @@ int f2fs_update_extension_list(struct f2fs_sb_info
>>> *sbi, const char *name,
>>> return -EINVAL;
>>>
>>> if (hot) {
>>> - strncpy(extlist[count], name, strlen(name));
>>> + memcpy(extlist[count], name, strlen(name));
>>
>> How about replacing with strcpy(extlist[count], name)? Because name length
>> has
>> already been checked before f2fs_update_extension_list, it should be valid,
>> and
>> will not cause overflow during copying.
>>
>
> Your call; feel free to submit an alternative. Since it is different files,
> static
> analysis might not know and complain, though. You might want to make sure
> that this
> doesn't happen, and also add a comment explaining the reason for using
> strcpy().
Yeah, that could be changed in another patch, but it will be trivial. Anyway, to
fix this gcc complaint, this patch looks good to me, thanks for the patch. :)
Reviewed-by: Chao Yu
Thanks,
>
> Thanks,
> Guenter
>
>
