The code below is part of my code to read CAN ID in Rx callback:
tmpp = (((0x07 << 0x1D)|(0x368 << 0x12)) & 0x1FFC0000); //unsigned long long int tmpp - equal to 0xDA00000
if (CAN0_IF2ARB0_ID == tmpp) {
//do some action
}
The problem is that while the 29 bit CAN ID is 0xDA00000, the condition is not true. But when I directly set tmpp as tmpp = 0xDA00000
, the program successfully enters the loop. In fact, the calculation tmpp = (((0x07 << 0x1D)|(0x368 << 0x12)) & 0x1FFC0000);
seems to have some problem (the value is 0xDA00000, but in Softune, it is not calculated correctly). I would be grateful if you could help me to find the problem. Thanks.
CodePudding user response:
0x07
is an int
- perhaps even a 16-bit int
. Use at least unsigned long
constants for values to be shifted into a 32-bit value.
// tmpp = (((0x07 << 0x1D)|(0x368 << 0x12)) & 0x1FFC0000);
tmpp = ((0x07ul << 0x1D) | (0x368ul << 0x12)) & 0x1FFC0000u;
CodePudding user response:
Left-shifting an integer constant such as 1
is almost always a bug, because integer constants in C have a type just like variables and in most cases it is int
. Now since int
is a signed type, we cannot left shift data into the sign bit or we invoke undefined behavior. 0x07 << 0x1D
does exactly that, it shifts data into bits 31 (sign bit), 30 and 29.
Solve this by always adding an u
suffix to all your integer constants.
Furthermore, you shouldn't use "magic numbers" but named constants. And in case you mean to shift something 29 bits, use decimal notation 29
since that's self-documenting code.
Your fixed code should look something like this (replace "MASKn" with something meaningful):
#define MASK1 (0x07u << 29)
#define MASK2 (0x368u << 18)
#define MASK3 (MASK1 | MASK2)
#define MASK4 0x1FFC0000u
if (CAN0_IF2ARB0_ID == (MASK3 & MASK4))
Also an extended CAN identifier doesn't use those bits 31,30,29... so I have no idea what you are even doing here. If you seek to calculate some value for CAN acceptance filtering etc, then it would seem you seem to have managed to confuse yourself by the original use of hex constants for shifting.