[FFmpeg-devel] [PATCH 1/5] lavc/htmlsubtitles: improve handling broken garbage

[James Almer]

On 7/30/2017 9:55 PM, Michael Niedermayer wrote:
> On Sun, Jul 30, 2017 at 03:47:39PM -0300, James Almer wrote:
>> On 7/30/2017 5:25 AM, Clément Bœsch wrote:
>>> On Sun, Jul 30, 2017 at 04:34:16AM +0200, Michael Niedermayer wrote:
>>> [...]
>>>>> +    struct font_tag stack[16] = {0};
>>>>
>>>> this seems to produce a compiler warning:
>>>>
>>>> ./libavcodec/htmlsubtitles.c: In function ‘ff_htmlmarkup_to_ass’:
>>>> ./libavcodec/htmlsubtitles.c:112:12: warning: missing braces around initializer [-Wmissing-braces]
>>>>
>>>
>>> Ah, I don't have that warning. Changed locally with a memset 0 (and of
>>> only the first element this time).
>>
>> You could try moving char "face[128]" to the end of the font_tag struct.
>> That should in theory also get rid of the warning.
>> Could you confirm that, Michael?.
> 
> seems this very minor warning issue led to a much bigger discussion
> than i expected ...
> 
> this is one way to fix the warning:
> -    struct font_tag stack[16] = {0};
> +    struct font_tag stack[16] = {{{0}}};
> 
> this is another:
> diff --git a/libavcodec/htmlsubtitles.c b/libavcodec/htmlsubtitles.c
> index 69d855df21..1950ddf54c 100644
> --- a/libavcodec/htmlsubtitles.c
> +++ b/libavcodec/htmlsubtitles.c
> @@ -65,9 +65,9 @@ static void handle_open_brace(AVBPrint *dst, const char **inp, int *an, int *clo
>  }
> 
>  struct font_tag {
> -    char face[128];
>      int size;
>      uint32_t color;
> +    char face[128];
>  };
> 
>  /*
> @@ -105,7 +105,7 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst, const char *in)
>       * remaining after the opening one was dropped. Yes, this happens and we
>       * still don't want to print a "</b>" at the end of the dialog event.
>       */
> -    struct font_tag stack[16] = {0};
> +    struct font_tag stack[16] = {{0}};
> 
>      for (; !end && *in; in++) {
>          switch (*in)
> 
> 
> I did not test these with other compilers, but i can if someone see
> value in it

No need. memset or av_mallocz are probably a better idea than any of
those two solutions.