[FFmpeg-devel] [RFC]lavu/timecode: Increase AV_TIMECODE_STR_SIZE.

[Clément Bœsch]

On Fri, May 05, 2017 at 09:44:46AM +0200, Carl Eugen Hoyos wrote:
> 2017-05-05 9:42 GMT+02:00 Aaron Levinson <alevinsn at aracnet.com>:
> > On 5/5/2017 12:20 AM, Carl Eugen Hoyos wrote:
> >>
> >> 2017-05-05 3:29 GMT+02:00 Aaron Levinson <alevinsn at aracnet.com>:
> >>>
> >>> On 5/4/2017 4:27 PM, Carl Eugen Hoyos wrote:
> >>>>
> >>>>
> >>>> Hi!
> >>>>
> >>>> Attached patch is one possibility to fix the following warning with gcc
> >>>> 7:
> >>>> libavutil/timecode.c: In function ‘av_timecode_make_string’:
> >>>> libavutil/timecode.c:103:60: warning: ‘%02d’ directive output may be
> >>>> truncated
> >>>> writing between 2 and 10 bytes into a region of size between 0 and 7
> >>>> [-Wformat-truncation=]
> >>>>      snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
> >>>>                                                             ^~~~
> >>>> libavutil/timecode.c:103:41: note: directive argument in the range [0,
> >>>> 2147483647]
> >>>>      snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
> >>>>                                          ^~~~~~~~~~~~~~~~~~~~~~~~
> >>>> libavutil/timecode.c:103:5: note: ‘snprintf’ output between 12 and 32
> >>>> bytes
> >>>> into a destination of size 16
> >>>>      snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
> >>>>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>>               neg ? "-" : "",
> >>>>               ~~~~~~~~~~~~~~~
> >>>>               hh, mm, ss, drop ? ';' : ':', ff);
> >>>>               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>>
> >>>> Several similar warnings are printed, an alternative is to disable the
> >>>> warning.
> >>>>
> >>>> The warning seemed wrong on first sight but may actually be correct, we
> >>>> accept invalid fps for timecode.
> >>>
> >>>
> >>>
> >>> Regarding the change in your patch:
> >>>
> >>> -#define AV_TIMECODE_STR_SIZE 16
> >>> +#define AV_TIMECODE_STR_SIZE 23
> >>>
> >>> It seems like 23 characters wouldn't be sufficient based on the warning:
> >>> "output between 12 and 32 bytes into a destination of size 16".  I would
> >>> guess that you would need at least 32 characters and perhaps one more (?)
> >>> for the terminating null character to avoid that warning.
> >>
> >>
> >> How can I reproduce / test your findings?
> >
> >
> > #define AV_TIMECODE_STR_SIZE 32 or 33 and confirm that gcc 7
> > warnings go away.
> 
> Of course!
> Since the warnings go away with 23, they also go away with 32.
> I just wonder why 23 isn't enough in your opinion?
> 

Because GCC says "‘snprintf’ output between 12 and 32 bytes".

32 bytes is fine with me, feel free to push with that value.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170505/bed15f26/attachment.sig>