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

[Arpi]

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.

> > >      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.

> > > -          uqvq = encoded[pixel_ptr++] + (encoded[pixel_ptr++] << 8);
> > > +          uqvq = encoded[pixel_ptr+1] + (encoded[pixel_ptr+2] << 8);
> > > +          pixel_ptr += 2;
> > 
> > i can't understand nor accept such changes
> 
> He's right here too. This is NOT valid C. It's undefined which ++
> happens first!

right, but the fix is bad again

pixel_ptr++ will increase _after_ evaluating its value, so it should be

+          uqvq = encoded[pixel_ptr] + (encoded[pixel_ptr+1] << 8);

but imho the best would be:
uqvq = encoded[pixel_ptr++]; uqvq += encoded[pixel_ptr++]<<8;
i do it this way in stream.h


A'rpi / Astral & ESP-team

--
Developer of MPlayer, the Movie Player for Linux - http://www.MPlayerHQ.hu