Evaluation if conditional statement on ST Visual Developer with Cosmic for STM8

Question

I am using STVD IDE to write code for STM8 the compiler I am using is Cosmic STM8 C compiler and the standard peripheral library provided for STM8.

The problem I have is while evaluating an if statement for an input pin, when evaluation the if statement with == 1 or == SET as shown below the condition is never met despite measuring high (3.3V) on the actual input itself

if(GPIO_ReadInputPin(GPIOC,GPIO_PIN_2) == SET)
if(GPIO_ReadInputPin(GPIOC,GPIO_PIN_2) == 1)

if the condition is == 0 or == RESET like the below code the code behaves properly, and the condition is evaluated successfully.

if(GPIO_ReadInputPin(GPIOC,GPIO_PIN_2) == RESET)
if(GPIO_ReadInputPin(GPIOC,GPIO_PIN_2) == 0)

I managed to make a workaround that by making the condition != 0 or != RESET as shown below it is evaluated successfully and the code behaves properly, but I don't really understand why it works with != 0 or != SET and not with == 0 or == RESET, does it has anything to do with the compiler?

if(GPIO_ReadInputPin(GPIOC,GPIO_PIN_2) != RESET)
if(GPIO_ReadInputPin(GPIOC,GPIO_PIN_2) != 0)

• the busybee: Welcome to StackOverflow! Please take the tour and read "How to Ask". -- Did you check the actual return value of GPIO_ReadInputPin(GPIOC,GPIO_PIN_2) when the pin is high? I'd expect it to be non-zero and not 1. Presumably it is the value of GPIO_PIN2. Please edit your question with the result of your debug/experiment, verifying or falsifying this assumption, and we will be happy to write an actual answer.
• Clifford: @busybee you are correct that the return value is GPIO_PIN2, but it is not unreasonable for the OP to expect SET or RESET since the return type is BitStatus an enum typedef with only those two values. Looking at the source, it is a long-standing bug in the library where the result of the bitmask is simply cast to BitStatus. I'd like to be surprised, but this is ST's code, and nothing surprises me anymore about that!

Answer 1

It is an undefined behaviour bug in GPIO_ReadInputPin() as described at https://community.st.com/t5/stm8-mcus/stm8-library-bug-gpio-readinputpin/td-p/490178

The implementation:

BitStatus GPIO_ReadInputPin(GPIO_TypeDef* GPIOx, GPIO_Pin_TypeDef GPIO_Pin) 
{
 return ((BitStatus)(GPIOx->IDR & (vu8)GPIO_Pin)); 
}

is severely flawed. It should be:

BitStatus GPIO_ReadInputPin(GPIO_TypeDef* GPIOx, GPIO_Pin_TypeDef GPIO_Pin) 
{
 return (GPIOx->IDR & (vu8)GPIO_Pin) == 0 ?
 RESET :
 SET;
}

for GPIO_PIN_2, the original requires the expression (BitSatus)Ox04 == SET, which will not happen. You cannot cast an integer to an enum successfully if that integer value is not represented in the enum, the result is undefined. Some compilers might coerce the value to one of the enumerated values, but I would expect the value to remain 0x04 and so not match either SET or RESET.

My advice in any event would be code defensively and always compare for inequality with zero or equivalent (such as RESET in this case). The reason being that a false is always zero while all non-zero values are regarded as true, making explicit comparison with 1 unsafe. So your final working solution would be the advised solution as it is proof against the kind of bug in this case which may be out of your control.

So in general, code defensively by using x != 0 or x == 0 (or simply x and !x with implicit conversion to Boolean) over x == 1 or x != 1 so that you only ever test for zero or non-zero rather than a specific non-zero value. Testing a known zero valued symbol such as RESET is OK, but unnecessary and another potential for error if you make the wrong assumptions about the symbol's zeroness.

Astonishingly, this issue remains uncorrected at https://documentation.help/STM8S-STM8A/stm8s__gpio_8c_source.html

• Amgad Bassam: Thank you that helped a lot and clears everything up.
• Clifford: You are welcome, but would remind you of stackoverflow.com/help/privileges/comment with respect to the When shouldn't I comment? part.