[MPlayer-dev-eng] [PATCH] Yet another batch of warning fixes :->

[Dominik Mierzejewski]

On Tuesday, 10 December 2002, Anders Johansson wrote:
> Hi,
> 
> > Hi,
> > 
> > > > >  #define ADDQUE(xi,xq,in)\
> > > > >    xq[xi]=xq[xi+L]=(*in);\
> > > > > -  xi=(--xi)&(L-1);
> > > > > +  xi=(xi-1)&(L-1);\
> > > > > +  xi--;
> > > > 
> > > > why?
> > > 
> > > He's correct that something's wrong here. Read the comp.lang.c faq.
> > > The result of an expression similar to x=--x; is undefined. I have no
> > ah. you're right
> > but then, that extra xi--; is unneeded and wrong
> > 
> > so, the right fix would be:
> > -  xi=(--xi)&(L-1);
> > +  xi=(xi-1)&(L-1);
> > 
> > IMHO, Anders should confirm.
> 
> It is the index for a circular buffer, the sulution Arpi suggests
> should work fine, but I haven't got time to test it right now.

Then why was xi decreased if it's not needed? My code does exactly the same
thing as the original one. I won't insist on my solution, though. My
intention was to tell you something was wrong there, and I've succeeded. :->

> > > > >      register int16_t* 	end   = in+ao_plugin_data.len/2;
> > > > > -    while(in < end) *in=(*in++)>>1;
> > > > > +    while(in < end) { *in=(*in)>>1; in++; }
> > > > 
> > > > again, why?
> > > 
> > > Same deal.
> > 
> > yes, but the fix is wrong.
> > it should be (IMHO!) *(++in)=(*in)>>1; or *(in+1)=*in>>1;++in;
> > 
> > IMHO the order that gcc (and other compilers) use is calculating the
> > rightvalue first, then storing the result in the left value.
> 
> use:
> while(in < end) { *in=(*in)>>1; in++;}

And that's exactly what I proposed.

-- 
MPlayer RPMs maintainer: http://www.piorunek.pl/~dominik/linux/pkgs/mplayer/
"The Universe doesn't give you any points for doing things that are easy."
        -- Sheridan to Garibaldi in Babylon 5:"The Geometry of Shadows"