* [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
@ 2023-10-29 12:47 Leo Izen
2023-10-29 12:49 ` Leo Izen
2023-10-30 0:23 ` Andreas Rheinhardt
0 siblings, 2 replies; 9+ messages in thread
From: Leo Izen @ 2023-10-29 12:47 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: 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 | 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);
}
}
--
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".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
2023-10-29 12:47 [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy Leo Izen
@ 2023-10-29 12:49 ` Leo Izen
2023-10-30 0:23 ` Andreas Rheinhardt
1 sibling, 0 replies; 9+ messages in thread
From: Leo Izen @ 2023-10-29 12:49 UTC (permalink / raw)
To: FFmpeg Development
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
2023-10-29 12:47 [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy Leo Izen
2023-10-29 12:49 ` Leo Izen
@ 2023-10-30 0:23 ` Andreas Rheinhardt
2023-10-30 0:33 ` Leo Izen
1 sibling, 1 reply; 9+ messages in thread
From: Andreas Rheinhardt @ 2023-10-30 0:23 UTC (permalink / raw)
To: ffmpeg-devel
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 | 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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
2023-10-30 0:23 ` Andreas Rheinhardt
@ 2023-10-30 0:33 ` Leo Izen
2023-10-30 7:51 ` Nicolas George
0 siblings, 1 reply; 9+ messages in thread
From: Leo Izen @ 2023-10-30 0:33 UTC (permalink / raw)
To: ffmpeg-devel
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 <leo.izen@gmail.com>
>> ---
>> 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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
2023-10-30 0:33 ` Leo Izen
@ 2023-10-30 7:51 ` Nicolas George
2023-10-30 13:52 ` Leo Izen
0 siblings, 1 reply; 9+ messages in thread
From: Nicolas George @ 2023-10-30 7:51 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
2023-10-30 7:51 ` Nicolas George
@ 2023-10-30 13:52 ` Leo Izen
2023-10-30 13:55 ` Nicolas George
0 siblings, 1 reply; 9+ messages in thread
From: Leo Izen @ 2023-10-30 13:52 UTC (permalink / raw)
To: ffmpeg-devel
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
2023-10-30 13:52 ` Leo Izen
@ 2023-10-30 13:55 ` Nicolas George
2023-10-30 14:01 ` Leo Izen
0 siblings, 1 reply; 9+ messages in thread
From: Nicolas George @ 2023-10-30 13:55 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
2023-10-30 13:55 ` Nicolas George
@ 2023-10-30 14:01 ` Leo Izen
2023-10-30 14:16 ` Nicolas George
0 siblings, 1 reply; 9+ messages in thread
From: Leo Izen @ 2023-10-30 14:01 UTC (permalink / raw)
To: ffmpeg-devel
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy
2023-10-30 14:01 ` Leo Izen
@ 2023-10-30 14:16 ` Nicolas George
0 siblings, 0 replies; 9+ messages in thread
From: Nicolas George @ 2023-10-30 14:16 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-10-30 14:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-29 12:47 [FFmpeg-devel] [PATCH v2] avformat/hls: use av_strlcopy instead of strncpy Leo Izen
2023-10-29 12:49 ` Leo Izen
2023-10-30 0:23 ` Andreas Rheinhardt
2023-10-30 0:33 ` Leo Izen
2023-10-30 7:51 ` Nicolas George
2023-10-30 13:52 ` Leo Izen
2023-10-30 13:55 ` Nicolas George
2023-10-30 14:01 ` Leo Izen
2023-10-30 14:16 ` Nicolas George
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git