On Sat, Jul 27, 2019 at 07:58:48AM -0700, Aaron Boushley wrote: > The vf_drawtext filter uses the GET_UTF8 macro in multiple locations. > Each of these use `continue;` as the error handler. However the > documentation for the GET_UTF8 macro states "ERROR should not contain > a loop control statement which could interact with the internal while > loop, and should force an exit from the macro code (e.g. through a > goto or a return) in order to prevent undefined results." > > This patch adjusts vf_drawtext to use goto error handlers similar to > other locations in ffmpeg. > > Aaron > > PS Sorry for having to send again, sent from the wrong address last > time, so patchwork didn't pick it up. >
> vf_drawtext.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
> 04550ed40f98ec78c2719de31da889e92ff42f45
> libavfilter-drawtext-avoid-undefined-behavior.patch
> From efdc96ace59d676e76434499a399d1d7df7fa093 Mon Sep 17 00:00:00 2001
> From: Aaron Boushley <boush...@gmail.com>
> Date: Fri, 26 Jul 2019 15:49:36 -0700
> Subject: [PATCH] libavfilter/drawtext: avoid undefined behavior with GET_UTF8
>
> Currently the GET_UTF8 usage in drawtext use a continue to skip invalid
> characters in a string. The macro definition states that using a loop
> control statement results in undefined behavior since the macro itself
> uses a loop.
>
> This switches drawtext to use a goto statement similar to other usages
> of GET_UTF8 in other parts of ffmpeg.
>
> Signed-off-by: Aaron Boushley <boush...@gmail.com>
> ---
> libavfilter/vf_drawtext.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
> index 8f4badbdb5..fd2ba84d12 100644
> --- a/libavfilter/vf_drawtext.c
> +++ b/libavfilter/vf_drawtext.c
> @@ -1223,7 +1223,7 @@ static int draw_glyphs(DrawTextContext *s, AVFrame
> *frame,
> for (i = 0, p = text; *p; i++) {
> FT_Bitmap bitmap;
> Glyph dummy = { 0 };
> - GET_UTF8(code, *p++, continue;);
> + GET_UTF8(code, *p++, goto invalid_drawing;);
>
> /* skip new line chars, just go to new line */
> if (code == '\n' || code == '\r' || code == '\t')
> @@ -1248,6 +1248,9 @@ static int draw_glyphs(DrawTextContext *s, AVFrame
> *frame,
> bitmap.width, bitmap.rows,
> bitmap.pixel_mode == FT_PIXEL_MODE_MONO ? 0 : 3,
> 0, x1, y1);
> + continue;
> +invalid_drawing:
> + av_log(s, AV_LOG_DEBUG, "Invalid UTF8 character while drawing
> glyphs\n");
> }
Why does this add av_log() calls ?
This seems unrelated to the bug fix. Also why are they debug level ?
if a user draws a string she provided that could be argued to be an error
the user wants to know about. But maybe iam missing something
Also they probably should provide more information about where the
invalid character is. It may be hard to find in a long text for some users
Thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The educated differ from the uneducated as much as the living from the
dead. -- Aristotle
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
