BITWISE.C integer types , bugs when compiling, typedef's
From BITWISE.C:
/* Read in bits without advancing the bitptr; bits <= 32 */
long oggpackB_look(oggpack_buffer *b,int bits){
unsigned long ret;
int m=32-bits;
bits+=b->endbit;
if(b->endbyte+4>=b->storage){
/* not the main path */
if(b->endbyte*8+bits>b->storage*8)return(-1);
}
ret=b->ptr[0]<<(24+b->endbit);
if(bits>8){
ret|=b->ptr[1]<<(16+b->endbit);
if(bits>16){
ret|=b->ptr[2]<<(8+b->endbit);
if(bits>24){
ret|=b->ptr[3]<<(b->endbit);
if(bits>32 && b->endbit)
ret|=b->ptr[4]>>(8-b->endbit);
}
}
}
return ((ret&0xffffffff)>>(m>>1))>>((m+1)>>1);
}
/* bits <= 32 */
long oggpackB_read(oggpack_buffer *b,int bits){
long ret;
long m=32-bits;
bits+=b->endbit;
if(b->endbyte+4>=b->storage){
/* not the main path */
ret=-1L;
if(b->endbyte*8+bits>b->storage*8)goto overflow;
}
ret=b->ptr[0]<<(24+b->endbit);
if(bits>8){
ret|=b->ptr[1]<<(16+b->endbit);
if(bits>16){
ret|=b->ptr[2]<<(8+b->endbit);
if(bits>24){
ret|=b->ptr[3]<<(b->endbit);
if(bits>32 && b->endbit)
ret|=b->ptr[4]>>(8-b->endbit);
}
}
}
ret=((ret&0xffffffffUL)>>(m>>1))>>((m+1)>>1);
overflow:
b->ptr+=bits/8;
b->endbyte+=bits/8;
b->endbit=bits&7;
return(ret);
}
The stuff seems to be very unstable ... in works with some compilers but not with other ones (fails self-test) :-(
-
Why does the upper function use UNSIGNED, but the lower one SIGNED INT32 for "ret" ? Wouldn't all integers UNSIGNED do the job better ? In my tests YES ...
-
Why do the bit counters (seem to allow values 0...32 only) use an SIGNED INT32 rather than an UINT8 ?
-
What is the difference between "long" and "int" ? Why aren't the TYPEDEF's from OS_TYPES.H used ?
typedef short ogg_int16_t;
typedef unsigned short ogg_uint16_t;
typedef int ogg_int32_t;
typedef unsigned int ogg_uint32_t;
typedef __int64 ogg_int64_t;
-
"ret&0xffffffffUL" vs "ret&0xffffffff" (no "UL") - intentional ?
-
x=(a>>b)>>c can be done better: x=a>>(b+c) ... except for b=c=16 ... but then 0 is returned :-( Is it useful to pick 0 bits ? If for some reason YES, it might be better to "short-circuit" return 0 for 0 bits, rather than process the complete magic.
-
What exactly is it supposed to do ? Some more detailed comments would be appreciated, "endbit" meaning ... Seems the sophisticated stuff does not do much more than just ONE (!) ASM instruction SHLD/SHRD ;-) So I could replace it with cca 5 lines of inline ASM.