[FFmpeg-devel] [PATCH]

[Aurelien Jacobs]

On Wed, 23 May 2007 21:48:42 +0100
M?ns Rullg?rd <mans at mansr.com> wrote:

> 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.

You're absolutely right. Good catch.

> >          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.

Hum... Ok. Let's try a totally different way. This one looks nicer to me.

Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: config-enable2.diff
Type: text/x-diff
Size: 1859 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070523/ee9d23e4/attachment.diff>