Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
Leo Izen (12023-10-30): > Also, fwiw, info->assoc_language is the name of a language. It shouldn't > actually ever be 63 characters long for real streams in the wild, and > truncating it is just a memory-safety precaution. It's not a bug. It SHOULD not, but it CAN be, and properly reporting the error to the user is important. It is just about adding an if and return error. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
On 10/30/23 09:55, Nicolas George wrote: Leo Izen (12023-10-30): difference: user data should not be silently truncated. There isn't really a bug here, just an extraneous compiler warning. Truncating user data silently is a bug. The warning is right and needs proper fix. Regards, The warning is not that truncation might occur, but that if it does occur, the buffer won't be nul-terminated. This is not actually the case because the size passed to strncpy is one smaller than the buffer size, and the buffer is zeroed. However, the compiler doesn't know this. Also, fwiw, info->assoc_language is the name of a language. It shouldn't actually ever be 63 characters long for real streams in the wild, and truncating it is just a memory-safety precaution. It's not a bug. - 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
Leo Izen (12023-10-30): > > difference: user data should not be silently truncated. > There isn't really a bug here, just an extraneous compiler warning. Truncating user data silently is a bug. The warning is right and needs proper fix. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
On 10/30/23 03:51, Nicolas George wrote: Leo Izen (12023-10-29): But it will be nul-terminated in either case, so there's no real difference between the old and new code, other than the warning. No real difference = you are not fixing the bug. There should be a difference: user data should not be silently truncated. There isn't really a bug here, just an extraneous compiler warning. - 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
Leo Izen (12023-10-29): > But it will be nul-terminated in either case, so > there's no real difference between the old and new code, other than the > warning. No real difference = you are not fixing the bug. There should be a difference: user data should not be silently truncated. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
On 10/29/23 20:23, Andreas Rheinhardt wrote:
Leo Izen:
Avoids a -Wstringop-truncation warning by using av_strlcopy instead of
strncpy.
Signed-off-by: Leo Izen
---
libavformat/hls.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavformat/hls.c b/libavformat/hls.c
index f5f549b24d..39440176c9 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -543,8 +543,8 @@ 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,
-sizeof(rend->language) - langlen - 2);
+av_strlcpy(rend->language + langlen + 1, info->assoc_language,
+sizeof(rend->language) - langlen - 1);
}
}
As I said before: You are merely hiding the truncation issue instead of
fixing it.
I don't see how? It will only be truncated if info->assoc_language is
very very long, and in that case it won't fit inside a 64-byte buffer so
it will *have* to be truncated. But it will be nul-terminated in either
case, so there's no real difference between the old and new code, other
than the warning.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] 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
> ---
> libavformat/hls.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index f5f549b24d..39440176c9 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -543,8 +543,8 @@ 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,
> -sizeof(rend->language) - langlen - 2);
> +av_strlcpy(rend->language + langlen + 1, info->assoc_language,
> +sizeof(rend->language) - langlen - 1);
> }
> }
>
As I said before: You are merely hiding the truncation issue 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
On 10/29/23 08:47, Leo Izen wrote: Avoids a -Wstringop-truncation warning by using av_strlcopy instead of strncpy. Changes from v1: - changed the length attribute, so now it has similar semantics to earlier. This patch has the same semantics with regard to the null-termination and the length of the string. However, it avoids a compiler warning because the compiler thinks strncpy is unsafe. - 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-requ...@ffmpeg.org with subject "unsubscribe".
