[FFmpeg-devel] [PATCH]

[Måns Rullgård]

Aurelien Jacobs <aurel at gnuage.org> writes:

> Hi,
>
> The attached patch will generate some #define ENABLE_* for every CONFIG_*
> items.
> Advantages:
>  - allow cleaner code, less ugly #ifdef (recent example with CONFIG_SWSCALER)
>  - allow to simplify configure removing some code duplication (see second
>    patch)
> I will apply soon if no one disagree.

The idea is OK, but see below.

> Index: configure
> ===================================================================
> --- configure	(revision 9102)
> +++ configure	(working copy)
> @@ -310,11 +310,17 @@
>      makefile=$3
>      shift 3
>      for cfg; do
> +        ucname="`toupper $cfg`"
>          if enabled $cfg; then
> -            ucname="${pfx}`toupper $cfg`"
> -            echo "#define ${ucname} 1" >> $header
> -            echo "${ucname}=yes" >> $makefile
> +            echo "#define ${pfx}${ucname} 1" >> $header
> +            echo "${pfx}${ucname}=yes" >> $makefile
> +            enable=1
> +        else
> +            enable=0

Some shells really hate variables and functions with the same name.  I
don't remember what POSIX says, but keeping them separate makes sense
anyway.  In short, please call that "enable" variable something else.

>          fi
> +        if [ "$pfx" = CONFIG_ ]; then
> +            echo "#define ENABLE_${ucname} $enable" >> $header
> +        fi
>      done
>  }

I don't quite like the look of this.  I'd prefer some less hardcoded
way of triggering the alternate define.

-- 
M?ns Rullg?rd
mans at mansr.com