| Message ID | 20231026031427.150182-1-leo.izen@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/hls: use av_strlcopy instead of strncpy | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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
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
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
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
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)
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); } }
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(-)