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

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

[Leo Izen]

Avoids a -Wstringop-truncation warning by using av_strlcopy instead of
strncpy.

Signed-off-by: Leo Izen <leo.izen@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);
         }
     }
-- 
2.42.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Steven Liu]

Leo Izen <leo.izen@gmail.com> 于2023年10月26日周四 11:14写道：
>
> Avoids a -Wstringop-truncation warning by using av_strlcopy instead of
> strncpy.
>
> Signed-off-by: Leo Izen <leo.izen@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);
>          }
>      }
> --
> 2.42.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

LGTM

Thanks
Steven
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Andreas Rheinhardt]

Leo Izen:
> Avoids a -Wstringop-truncation warning by using av_strlcopy instead of
> strncpy.
> 
> Signed-off-by: Leo Izen <leo.izen@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

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[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@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.

- Leo Izen
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [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@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

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Leo Izen]

On 10/26/23 07:32, Andreas Rheinhardt wrote:
> 
> 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
> 

Truncation can happen if info->assoc_language is very long, i.e. uses 
the full buffer (63 chars plus a 64th for nul).

In that case, strncpy will truncate, but because there's a -2 in the 
size argument, instead of -1, the resulting buffer will still be 
nul-terminated. The compiler doesn't know this, so it produces a warning.

It seems the appropriate fix here is to change strncpy to av_strlcpy but
also change the -2 to a -1. This will produce identical behavior to 
before, but it won't cause the compiler to produce a warning that the 
string might not be nul-terminated (cause it will always be).

- Leo Izen (Traneptora)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".