Describe the bug 24 bits Integer saving is error。
To reproduce
void zipSaveInteger(unsigned char *p, int64_t value, unsigned char encoding) {
.......
else if (encoding == ZIP_INT_24B) {
// if LBS
//
// If value is 0x00112233； in memory : 33 22 11 00
i32 = value<<8; //--> in memory 00 33 22 11
memrev32ifbe(&i32); //--> in memory 11 22 33 00
// ???? I see “((uint8_t*)&i32)+1” is bug
memcpy(p,((uint8_t*)&i32)+1,sizeof(i32)-sizeof(uint8_t));
}
.......
}
Expected behavior
Additional information
Comment From: oranagra
@damagao are you using / referring to a bigendian system? from your comments, it appears that you don't, but then memrev32ifbe should be a NOP (doesn't change the variable). maybe you missed something?
from what i could tell, for 24 bit numbers in 32bit variables, the most significant byte would be 0, and in LE that's the last (higher address) byte.
so doing <<8, would mean the least significant (lowest address byte) is now 0, which is why we skip it with the +1.
i admit the code is convoluted, but it doesn't seem buggy. on the bright side, we're no longer using it (redis 7.0 shifted away from ziplists to listpacks). but still, if we could learn that there's a bug in it, we rather know about it and realize it's implications...
Comment From: damagao
Example: value is 0x00112233 ; in memory(address low to hight ）by LBS 33 22 11 00 i32 = value<<8; // value is in memory 00 33 22 11 memrev32ifbe(&i32); // value is in memory 11 22 33 00 memcpy(p,((uint8_t)&i32)+1,sizeof(i32)-sizeof(uint8_t)); // (uint8_t)&i32) point to the address of value of 11
Do you see my mease?
Comment From: oranagra
Again in little endian systems, memrev32ifbe is NOP. For some reason you show as if to reverses the byte order. Is that what you're missing? Maybe you should write a small test that prints the results..