Shifting integers in C is easy. Alas it is also easy to get it wrong. A common issue found using static analysis on the Linux kernel is the unintentional sign extension gotcha.
Consider the following code that takes the 4 unsigned 8 bit integers in array data and returns an unsigned 64 bit integer:
C promotes the uint8_t integers into signed ints on the right shift. If data[3] has the upper bit set, for example with the value 0x80 and data[2]..data[0] are zero, the shifted 32 bit signed result is sign extended to a 64 bit long and the final result is 0xffffffff80000000. A way to fix this is to explicitly cast data[3] to a uint64_t before shifting it.
Fortunately static analysis finds these issues. Unfortunately it is a bug that keeps on occurring in new code.
Since the intent of the 'ret =' line is to construct a 32-bit value from the four bytes, a more intuitive fix might be to simply declare ret as a uint32_t instead (or use a uint32_t temporary to hold the constructed value if the upper 32 bits of ret are going to be filled later in the function.
ReplyDeleteI cast anything I want to shift to the target type (or larger type) first, this avoids any headache.
ReplyDelete