[MPlayer-dev-eng] squelch some warnings (patches incl.)

Diego Biurrun diego at biurrun.de
Mon Feb 25 16:58:16 CET 2008


On Mon, Feb 25, 2008 at 03:20:20AM +0100, Michael Niedermayer wrote:
> On Mon, Feb 25, 2008 at 12:11:41AM +0100, Diego Biurrun wrote:
> > On Sun, Feb 24, 2008 at 08:25:33PM +0100, Elias Pipping wrote:
> > > 
> > > On a sidenote (replying here because Michael Niedermayer did not cc me):
> > > 
> > > libavcodec/snow.c basically looks like this:
> > > 
> > >   #define QUANTIZE2 0
> > >   #if QUANTIZE2==1
> > >   static void dwt_quantize(<args>){
> > >     <implementation>
> > >   }
> > >   #endif
> > > 
> > >   if(QUANTIZE2)
> > >     dwt_quantize(<args>);
> > > 
> > > The `#if QUANTIZE2==1` is effectively a `#if 0`. Hence, dwt_quantize()
> > > is never declared. I don't see a reason to check for QUANTIZE2 at
> > > runtime when it's already clear at compile-time that dwt_quantize() will
> > > never be called. 
> 
> It is not checked at runtime, gcc will remove this check at compiletime
> 
> 
> > > The fact that the definition of dwt_quantize() is
> > > removed by the preprocessor but the invocation stays in, is what causes
> > > the warning and my patch fixes that.
> > > Please let me know what made you
> > > reject it.
> > 
> > I have to agree with Elias here.  The fix is not uglier than the
> > original code...
> 
> First i must say that iam VERY alergic to cleanup of experimental code
> on which iam working and which will possibly be removed in the end.
> And iam also alergic to having to explain why i dont like
> "my hammer being moved around while i work with it".
> 
> Now ive rewritten this reply a few times in the hope that it wont sound
> to harsh ... ohh well all my beautifull insults and the threat of moving
> snow.c to a repo to which only i have write access ... ohh well ;)

:)

> Cleanup to snow is welcome, some examples:
> * de-mplayerify indention
> * remove START/STOP_TIMER (they arent ever where i want them anyway)
> * normalize {} placement
> * remove all the outcommented av_log()

So if I do all of the above, will you fix the two remaining warnings in
snow.c?  Sure sounds like a good deal to me ...

> Also, if you think if(<this will be 0>) something_nonexistant is ugly
> look at some commits from aurel, like allcodecs.c. And fix this, which
> frankly would be identical to reverting aurels work and iam pretty sure
> he will like this as much as i do like these changes to snow.c.

I did not say what Aurel did was ugly :)

Diego



More information about the MPlayer-dev-eng mailing list