[MPlayer-cvslog] r31584 - trunk/libmpcodecs/vd_ffmpeg.c

[Michael Niedermayer]

On Mon, Jun 28, 2010 at 10:45:31PM +0200, Diego Biurrun wrote:
> On Mon, Jun 28, 2010 at 10:18:05PM +0200, Michael Niedermayer wrote:
> > On Mon, Jun 28, 2010 at 06:54:28PM +0200, diego wrote:
> > > 
> > > Log:
> > > Place swap_palette declaration under the same #ifdef as its usage; fixes:
> > > libmpcodecs/vd_ffmpeg.c:784: warning: 'swap_palette' defined but not used
> > > 
> > > --- trunk/libmpcodecs/vd_ffmpeg.c	Mon Jun 28 18:45:47 2010	(r31583)
> > > +++ trunk/libmpcodecs/vd_ffmpeg.c	Mon Jun 28 18:54:27 2010	(r31584)
> > > @@ -780,6 +780,7 @@ typedef struct dp_hdr_s {
> > >  
> > > +#if HAVE_BIGENDIAN
> > >  static void swap_palette(void *pal)
> > >  {
> > >      int i;
> > 
> > please use av_unused or inline
> 
> Why?

av_unused
    means that the function is possibly unused
#if HAVE_BIGENDIAN
    means that the code is specific to big endian
The code is as much specific to big endian as printf()
It maybe currently be just used on big endian but the code is not specific
to big endian.
Also the ifdefery is fragile. if one change the code to use it in cases
outside HAVE_BIGENDIAN it will break and that may be limited to a small
number of systems.
Using HAVE_BIGENDIAN like that is just not professionally written C code.
Its almost like hardcoding a constant at 2 distant places and expecting by
magic that noone would change one while missing the other.
also with av_unused the code is compile tested everywhere with HAVE_BIGENDIAN
it is not and can fail easier.
then av_unused is 1 line less

and iam sure one could come up with more if one tried

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-cvslog/attachments/20100629/e347e51c/attachment.pgp>