[FFmpeg-devel] Re: [PATCH] avutil/bprint: fix av_bprint_strftime with %p format string reporting truncated output (PR #20330)

[Nicolas George via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

Marton Balint via ffmpeg-devel (HE12025-08-24):
> >From 4532caf820dcb2bc2cbac3d4e782ad27cf9fd76b Mon Sep 17 00:00:00 2001
> From: Marton Balint <cus@passwd.hu>
> Date: Sun, 24 Aug 2025 21:35:41 +0200
> Subject: [PATCH 2/2] avutil/bprint: fix av_bprint_strftime with %p format
>  string reporting truncated output
> 
> strftime returns 0 in case of an empty output (e.g. %p format string with some
> locales), there is no way to distinguish this from a buffer-too-small error
> condition. So we must use some heuristics to handle this case, and not consume
> INT_MAX RAM and falsely report a truncated output.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavutil/bprint.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/libavutil/bprint.c b/libavutil/bprint.c
> index 932c03ce50..fa244b2e04 100644
> --- a/libavutil/bprint.c
> +++ b/libavutil/bprint.c
> @@ -167,6 +167,7 @@ void av_bprint_strftime(AVBPrint *buf, const char *fmt, const struct tm *tm)
>  {
>      unsigned room;
>      size_t l;

> +    size_t fmt_len = 0;

Why not set it here once and for all?

>  
>      if (!*fmt)
>          return;
> @@ -174,9 +175,16 @@ void av_bprint_strftime(AVBPrint *buf, const char *fmt, const struct tm *tm)
>          room = av_bprint_room(buf);
>          if (room && (l = strftime(buf->str + buf->len, room, fmt, tm)))
>              break;
> +
> +        if (!fmt_len)
> +            fmt_len = strlen(fmt);
> +        /* A 256x space requirement for output is unlikely, so it is likely empty */
> +        if (room >> 8 > fmt_len)
> +            break;
> +

IIUC, at this point l=0 means either the buffer is too small or the
output is empty, and you distinguish between the cases by checking that
there is plenty of room, 256 chars for any char in the buffer?

I think a lower margin should be ok, >>4 or >>5.

OTOH, I think a comment would be a good idea.

>          /* strftime does not tell us how much room it would need: let us
>             retry with twice as much until the buffer is large enough */
> -        room = !room ? strlen(fmt) + 1 :
> +        room = !room ? fmt_len + 1 :
>                 room <= INT_MAX / 2 ? room * 2 : INT_MAX;
>          if (av_bprint_alloc(buf, room)) {
>              /* impossible to grow, try to manage something useful anyway */

Or we could write our own strftime, not localized.

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org