[FFmpeg-devel] lavf/mlv: Fix an snprintf() truncation

Hi!

Attached patch fixes a warning when compiling with gcc 7:
libavformat/mlvdec.c: In function ‘read_header’:
libavformat/mlvdec.c:353:58: warning: ‘snprintf’ output may be truncated 
before the last format character [-Wformat-truncation=]
             snprintf(filename + strlen(filename) - 2, 3, "%02d", i);

Please comment, Carl Eugen
From f56bf75b2b8b99cbbe99da8d2e33e46bf50be92d Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <cehoyos@ag.or.at>
Date: Tue, 9 May 2017 15:27:44 +0200
Subject: [PATCH] lavf/mlvdec: Avoid snprintf() output truncation.

Fixes a gcc warning:
'snprintf' output may be truncated before the last format character
---
 libavformat/mlvdec.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Tue, May 09, 2017 at 03:32:36PM +0200, Carl Eugen Hoyos wrote:
> Hi!
> 
> Attached patch fixes a warning when compiling with gcc 7:
> libavformat/mlvdec.c: In function ‘read_header’:
> libavformat/mlvdec.c:353:58: warning: ‘snprintf’ output may be truncated 
> before the last format character [-Wformat-truncation=]
>              snprintf(filename + strlen(filename) - 2, 3, "%02d", i);
> 
> Please comment, Carl Eugen

> From f56bf75b2b8b99cbbe99da8d2e33e46bf50be92d Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <cehoyos@ag.or.at>
> Date: Tue, 9 May 2017 15:27:44 +0200
> Subject: [PATCH] lavf/mlvdec: Avoid snprintf() output truncation.
> 
> Fixes a gcc warning:
> 'snprintf' output may be truncated before the last format character
> ---
>  libavformat/mlvdec.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/mlvdec.c b/libavformat/mlvdec.c
> index 319cd26..372acbe 100644
> --- a/libavformat/mlvdec.c
> +++ b/libavformat/mlvdec.c
> @@ -349,7 +349,7 @@ static int read_header(AVFormatContext *avctx)
>          if (!filename)
>              return AVERROR(ENOMEM);
>  
> -        for (i = 0; i < 100; i++) {
> +        for (i = 0; i < 99; i++) {
>              snprintf(filename + strlen(filename) - 2, 3, "%02d", i);
>              if (avctx->io_open(avctx, &mlv->pb[i], filename, AVIO_FLAG_READ, NULL) < 0)
>                  break;

can you explain? The current loop is not supposed to ever reach 100.

2017-05-09 16:40 GMT+02:00 Clément Bœsch <u@pkh.me>:
> On Tue, May 09, 2017 at 03:32:36PM +0200, Carl Eugen Hoyos wrote:
>> Hi!
>>
>> Attached patch fixes a warning when compiling with gcc 7:
>> libavformat/mlvdec.c: In function ‘read_header’:
>> libavformat/mlvdec.c:353:58: warning: ‘snprintf’ output may be truncated
>> before the last format character [-Wformat-truncation=]
>>              snprintf(filename + strlen(filename) - 2, 3, "%02d", i);
>>
>> Please comment, Carl Eugen
>
>> From f56bf75b2b8b99cbbe99da8d2e33e46bf50be92d Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos <cehoyos@ag.or.at>
>> Date: Tue, 9 May 2017 15:27:44 +0200
>> Subject: [PATCH] lavf/mlvdec: Avoid snprintf() output truncation.
>>
>> Fixes a gcc warning:
>> 'snprintf' output may be truncated before the last format character
>> ---
>>  libavformat/mlvdec.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavformat/mlvdec.c b/libavformat/mlvdec.c
>> index 319cd26..372acbe 100644
>> --- a/libavformat/mlvdec.c
>> +++ b/libavformat/mlvdec.c
>> @@ -349,7 +349,7 @@ static int read_header(AVFormatContext *avctx)
>>          if (!filename)
>>              return AVERROR(ENOMEM);
>>
>> -        for (i = 0; i < 100; i++) {
>> +        for (i = 0; i < 99; i++) {
>>              snprintf(filename + strlen(filename) - 2, 3, "%02d", i);
>>              if (avctx->io_open(avctx, &mlv->pb[i], filename, AVIO_FLAG_READ, NULL) < 0)
>>                  break;
>
> can you explain?

No, the following allows to reproduce the warning with "-O3 -Wformat"
and gcc 7 both
with and without the commented code:

void foo()
{
unsigned i = 0;
char filename[]="ab";
    while (++i < 100)
        snprintf(filename /*+ strlen(filename) - 2*/, 3, "%02u", i);
}

Carl Eugen

On Wed, May 10, 2017 at 09:49:50AM +0200, Carl Eugen Hoyos wrote:
> 2017-05-09 16:40 GMT+02:00 Clément Bœsch <u@pkh.me>:
> > On Tue, May 09, 2017 at 03:32:36PM +0200, Carl Eugen Hoyos wrote:
> >> Hi!
> >>
> >> Attached patch fixes a warning when compiling with gcc 7:
> >> libavformat/mlvdec.c: In function ‘read_header’:
> >> libavformat/mlvdec.c:353:58: warning: ‘snprintf’ output may be truncated
> >> before the last format character [-Wformat-truncation=]
> >>              snprintf(filename + strlen(filename) - 2, 3, "%02d", i);
> >>
> >> Please comment, Carl Eugen
> >
> >> From f56bf75b2b8b99cbbe99da8d2e33e46bf50be92d Mon Sep 17 00:00:00 2001
> >> From: Carl Eugen Hoyos <cehoyos@ag.or.at>
> >> Date: Tue, 9 May 2017 15:27:44 +0200
> >> Subject: [PATCH] lavf/mlvdec: Avoid snprintf() output truncation.
> >>
> >> Fixes a gcc warning:
> >> 'snprintf' output may be truncated before the last format character
> >> ---
> >>  libavformat/mlvdec.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavformat/mlvdec.c b/libavformat/mlvdec.c
> >> index 319cd26..372acbe 100644
> >> --- a/libavformat/mlvdec.c
> >> +++ b/libavformat/mlvdec.c
> >> @@ -349,7 +349,7 @@ static int read_header(AVFormatContext *avctx)
> >>          if (!filename)
> >>              return AVERROR(ENOMEM);
> >>
> >> -        for (i = 0; i < 100; i++) {
> >> +        for (i = 0; i < 99; i++) {
> >>              snprintf(filename + strlen(filename) - 2, 3, "%02d", i);
> >>              if (avctx->io_open(avctx, &mlv->pb[i], filename, AVIO_FLAG_READ, NULL) < 0)
> >>                  break;
> >
> > can you explain?
> 
> No, the following allows to reproduce the warning with "-O3 -Wformat"
> and gcc 7 both
> with and without the commented code:
> 
> void foo()
> {
> unsigned i = 0;
> char filename[]="ab";
>     while (++i < 100)
>         snprintf(filename /*+ strlen(filename) - 2*/, 3, "%02u", i);
> }

it sounds like a GCC bug then, the patch is wrong.

note: this is not the same loop as mlvdec, your example is [1:99], pre
patch mlvdec is [0:99].