[FFmpeg-devel] [PATCH] avformat/hls: use av_strlcopy instead of strncpy

[Andreas Rheinhardt]

Leo Izen:
> On 10/26/23 06:54, Andreas Rheinhardt wrote:
>> Leo Izen:
>>> Avoids a -Wstringop-truncation warning by using av_strlcopy instead of
>>> strncpy.
>>>
>>> Signed-off-by: Leo Izen <leo.izen at gmail.com>
>>> ---
>>>   libavformat/hls.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>>> index f5f549b24d..076f92ecfb 100644
>>> --- a/libavformat/hls.c
>>> +++ b/libavformat/hls.c
>>> @@ -543,7 +543,7 @@ static struct rendition *new_rendition(HLSContext
>>> *c, struct rendition_info *inf
>>>           int langlen = strlen(rend->language);
>>>           if (langlen < sizeof(rend->language) - 3) {
>>>               rend->language[langlen] = ',';
>>> -            strncpy(rend->language + langlen + 1, info->assoc_language,
>>> +            av_strlcpy(rend->language + langlen + 1,
>>> info->assoc_language,
>>>                       sizeof(rend->language) - langlen - 2);
>>>           }
>>>       }
>>
>> Doesn't this just silence the warning instead of fixing the potential
>> truncation?
>>
>> - Andreas
>>
> 
> The semantics of strlcpy and strncpy are slightly different. strlcopy
> *always* nul-terminates the destination string. strncpy zeroes the
> buffer and then runs memcpy, so if it would overflow the buffer the
> string ends up without a nul-terminator. The warning triggers because
> the compiler thinks that case can occur.
> 

This case can't happen here, because it only copies
"sizeof(rend->language) - langlen - 2" bytes at most from position where
sizeof(rend->language) - langlen - 1 are available, so the initial
trailing \0 never gets touched. You did not touch the size part of the
call, so you effectively only use sizeof(rend->language) - 1 bytes of
the buffer, thereby making the truncation issue worse. And even if you
fixed this part, you would still just have silenced the truncation
instead of fixing it.

- Andreas