[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