[FFmpeg-cvslog] r15002 - in trunk/libavformat: internal.h sdp.c utils.c

[Stefano Sabatini]

On date Saturday 2008-08-30 02:16:26 +0200, Aurelien Jacobs wrote:
> Stefano Sabatini wrote:
> 
> > On date Friday 2008-08-29 01:20:00 +0200, Stefano Sabatini wrote:
> > > On date Friday 2008-08-29 00:28:06 +0200, Diego Biurrun wrote:
> > > > On Thu, Aug 28, 2008 at 03:01:26AM +0200, Aurelien Jacobs wrote:
> > > > > M?ns Rullg?rd wrote:
> > > > > 
> > > > > > Aurelien Jacobs <aurel at gnuage.org> writes:
> > > > > > 
> > > > > > > rbultje wrote:
> > > > > > >
> > > > > > >> --- (empty file)
> > > > > > >> +++ trunk/libavformat/internal.h	Thu Aug 28 01:43:28 2008
> > > > > > >> @@ -0,0 +1,26 @@
> > > > > > >> +
> > > > > > >> +#ifndef FFMPEG_AVFORMAT_UTILS_H
> > > > > > >> +#define FFMPEG_AVFORMAT_UTILS_H
> > > > > > >
> > > > > > > Sorry that I didn't catch this when you submitted the patch, but
> > > > > > > here, the multiple inclusion guard should be FFMPEG_INTERNAL_H.
> > > > > > > But now that I think about it. This would cause clashes with
> > > > > > > the multiple inclusion guard from lavu/interal.h.
> > > > > > > I guess that was one of the reason why I preferred AVFORMAT_FILE_H
> > > > > > > as a standard inclusion guard instead of FFMPEG_FILE_H at the time
> > > > > > > this was decided.
> > > > > > > Should we consider a new rename of all inclusion guards ?
> > > > > > 
> > > > > > It was Diego who renamed them all to FFMPEG_*.  I never understood his
> > > > > > reasoning behind it.
> > > > > 
> > > > > So would you agree renaming all of them to AVFORMAT_*, AVCODEC_*, etc ?
> > > > > Would anyone be against it ?
> > > > 
> > > > I think we/I chose FFMPEG_ for simplicity back then.  I agree that
> > > > AVCODEC_ etc. is the better solution in the long run.
> > > 
> > > I'm all for it too, it would avoid potentially nasty-to-debug issues.
> > > 
> > > A simple script could do the work.
> > 
> > Script and patch attached.
> 
> Argh... Just when I was working on the same thing...
> 
> > Rule applied to transform filename in the guard name:
> > libavfoo/foo/bar/one_two_three.h -> AVFOO_FOO_BAR_ONE_TWO_THREE_H
> 
> I applied the exact same rule.
> And just to be complete, files in the root directory uses FFMPEG_*_H.

Yes they're already correct so there is no need to change them.

> Attached the resulting diff which should be almost the same as yours
> with a few things ajusted by hand (eg: libavformat/avc.h had a wrong
> guard and wasn't changed by your patch).
> As no-one seems to disagree, I propose to apply it soon.

I'll apply it tomorrow on evening or on Sunday if no one has
objections.
 
> > [...long perl script...]
> 
> Nice long code.
> This small shell script should do the same:
> 
> for file in `find . -name '*.h' | sed s%^./%%`; do
>   prefix=`echo $file | grep -q / || echo FFMPEG_`
>   def=`grep '^#ifndef FFMPEG_.*_H$' $file | sed 's/#ifndef //'`
>   newdef=$prefix`echo $file|sed s/^lib//|tr [a-z/.] [A-Z__]`
>   if [ -n "$def" -a "$def" != $newdef ]; then
>     echo s/$def/$newdef/ $file
>   fi
> done
> 
> And more interesting, here is a samll script which check the
> conformance of all the .h files and return a list of the wrong
> ones. This could be used in a pre-commit hook or FATE test or
> whatever...
> 
> for file in `find . -name '*.h' | sed s%^./%%`; do
>   prefix=`echo $file | grep -q / || echo FFMPEG_`
>   def=$prefix`echo $file|sed s/^lib//|tr [a-z/.] [A-Z__]`
>   grep -q "#ifndef $def"  $file && \
>   grep -q "#define $def"  $file && \
>   grep -q "#endif .*$def" $file || \
>   grep -q 'intentionally has no multiple inclusion guards' $file || \
>   test $file == version.h || \
>   echo $file
> done

Regards.