[vlc-devel] [PATCH] demux/xiph_metadata.c: fix bug causing redudant "extra" metadata

[Rémi Denis-Courmont]

On Tuesday 17 May 2016 18:35:34 Filip Roséen wrote:
> The IF_EXTRACT_FMT macro is to be used in a if-else-tree, the usage of
> IF_EXTRACT prior to the following if-statement did however cause the
> single if-else-tree to be split up into two (causing the fallback-else
> at the end to be unconditionally hit, even if another branch had already
> handled the data).
> 
> This patch fixes that issue.
> 
> --
> 
> Sample usage (from xiph_metadata.c):
> 
>         IF_EXTRACT("TITLE=", Title )
>         ...
>         else IF_EXTRACT_ONCE("TRACKTOTAL=", TrackTotal )
>         else IF_EXTRACT_ONCE("TOTALTRACKS=", TrackTotal )
>         else IF_EXTRACT("DESCRIPTION=", Description )
>         else IF_EXTRACT("COMMENT=", Description )
>         else IF_EXTRACT("COMMENTS=", Description )
>         else IF_EXTRACT("RATING=", Rating )
>         else IF_EXTRACT("DATE=", Date )
>         else IF_EXTRACT_FMT("LANGUAGE=", Language, p_fmt, psz_language )
>         ...
>         else if( ... /* fallback */ ) {
>             vlc_meta_AddExtra( ... );
>         }
> 
> If "TITLE=" metadata was found, we would still end up inside the
> fallback since the previous implementation of IF_EXTRACT_FMT would start
> a new if-else-tree.
> ---
>  modules/demux/xiph_metadata.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/modules/demux/xiph_metadata.c b/modules/demux/xiph_metadata.c
> index 2e9ecba..f8878ad 100644
> --- a/modules/demux/xiph_metadata.c
> +++ b/modules/demux/xiph_metadata.c
> @@ -389,12 +389,15 @@ void vorbis_ParseComment( es_format_t *p_fmt,
> vlc_meta_t **pp_meta, }
> 
>  #define IF_EXTRACT_FMT(txt,var,fmt,target) \
> -    IF_EXTRACT(txt,var)\
> -    if( fmt && !strncasecmp(psz_comment, txt, strlen(txt)) )\
> +    if( !strncasecmp(psz_comment, txt, strlen(txt)) ) \
> +    { \
> +        IF_EXTRACT(txt,var)\
> +        if( fmt )\
>          {\
>              if ( fmt->target ) free( fmt->target );\

Remove the if() while you´re at it.

>              fmt->target = strdup(&psz_comment[strlen(txt)]);\
> -        }
> +        }\
> +    }
> 
>          IF_EXTRACT("TITLE=", Title )
>          else IF_EXTRACT("ARTIST=", Artist )

-- 
Rémi Denis-Courmont
http://www.remlab.net/