drivers: gpio: Use unsigned integers for bitwise operations #89868
Conversation
According to the C11 standard, section 6.5, paragraph 4, the usage of bitwise operators on signed integers is implementation defined. So only unsigned integer constants shall be used for bitwise shifting. Signed-off-by: Stephan Gatzka <stephan.gatzka@gmail.com>
|
|
|
||
| /** Enables pin as input. */ | ||
| #define GPIO_INPUT (1U << 16) | ||
| #define GPIO_INPUT (1U << 16U) |
There was a problem hiding this comment.
I'm not sure this is correct. godbolt says that unsigned shifted by int is still unsigned:
https://godbolt.org/z/MG9on18Ws
And this reference:
says that the resulting type is the result of promoting (unsigned, int) to a common type, which is unsigned.
I also can't find any other use of this in Zephyr.
So I don't think this removes undefined behaviour, but it does make it mildly harder to read.
There was a problem hiding this comment.
Well, both links doesn't really help. It just shows how a specific compiler behaves, therefore "implementation defined".
There was a problem hiding this comment.
Sure, for the first, but the second is Microsoft's summary of the standard, not their implementation.
Here's the relevant section from cppreference:
https://en.cppreference.com/w/cpp/language/operator_arithmetic "Built-in bitwise shift operators": "Integral promotions are performed on both operands." and "The type of the result is that of lhs."
I couldn't find a copy of the C spec to reference.
henrikbrixandersen
left a comment
henrikbrixandersen
left a comment
There was a problem hiding this comment.
All of these should be changed to use the BIT() macro instead.
That makes sense. I'l cancel this PR and make a new one. |
3 participants
According to the C11 standard, section 6.5, paragraph 4, the usage of bitwise operators on signed integers is implementation defined. So only unsigned integer constants shall be used for bitwise shifting.