[theora-dev] bitpack.c odditiy

[Timothy B. Terriberry]

Nils Pipenbrinck <n.pipenbrinck at cubic.org> wrote:

> While browsing the code I came across line 83 bitpack.c:
>
>   *_ret=((ret&0xFFFFFFFFUL)>>(m>>1))>>(m+1>>1);
>
> Is there any reason why this is so convoluted? Maybe endianess or 64 bit
> issues? If I'm not mistaken it does exactly the the same as:
>
>   *_ret = ret >> m;

Yes, there is. 64-bit issues, as you noted, require the mask. It  
should be optimized out by a 32-bit compiler. However, m can take the  
full range of values from 0 to 32. Shifting by values as large as the  
word size is not defined for ANSI C, and on Intel machines in  
particular, a shift by 32 (of a 32-bit value) is treated as a shift by  
zero (it only considers the lower 5 bits of the shift index). The  
above code correctly returns zero (i.e., it shifts off all 32 bits).