Bringing to a larger type in the switch housing
While working, I saw the following piece of code, and now I wonder if there is a reason for distinguishing "case-values" to a larger data type. I assume it is being used to offer the ability to have more than 256 different states later, but then the state variable must be larger as well. Here is the code I'm talking about:
#define STATE_1 (uint8_t)(0x00)
#define STATE_2 (uint8_t)(0x01)
...
#define STATE_n (uint8_t)(0x..)
void HandleState(uint8_t state)
{
switch(state)
{
case (uint16_t)STATE_1:
// handle state 1
break;
case (uint16_t)STATE_2:
// handle state 2
break;
...
case (uint16_t)STATE_n:
// handle state n
break;
default:
break;
}
}
Is there another reason for this?
source to share
It looks like a slightly unfortunate attempt to get rid of implicit whole promotions on an 8-bit or 16-bit microcontroller.
First of all, there can be no targeted advertising here, so prevention of over-pedantry and ease of reading. At first glance, this looks like nonsense.
But.
Perhaps the coding standard being used enforces the use of implicit conversions? That, for example, MISRA-C. In this case, the code suddenly makes sense: they would like to suppress warnings from their static analysis tool.
Using explicit casts would then also be a way to demonstrate that you are indeed aware of implicit promotion types (something that only a few C programmers print) and that you handle them in your code.
But if so, the programmer missed one whole promotion, as follows: switch(state)
. This is how switch statements work:
Integer promotions are executed over the control expression. A constant expression in each case, the label is converted to the promoted type of the control expression.
So, if the programmer was concerned about implicit shares, they had to write the code as switch((uint16_t)state)
and then also store the casts in each one case
.
If you dodged the code in the question, consider the following:
- Do you really know what hidden promotions are in a switch statement, and have you considered how they might affect this code?
- Do you know the meaning of the whole promotion rules? Do you know and think what
uint8_t
will get in this code (signed)int
? - Are you using the coding standard and static analysis tools yourself?
source to share
This sounds like bad style. You must have one typedef for the state:
typedef uint8_t State;
Then your code will look like this:
#define STATE_1 (State)(0x00)
#define STATE_2 (State)(0x01)
...
#define STATE_n (State)(0x..)
void HandleState(State state)
{
switch(state)
{
case (State)STATE_1:
// handle state 1
break;
case (State)STATE_2:
// handle state 2
break;
...
case (State)STATE_n:
// handle state n
break;
default:
break;
}
}
Then, if you need to uint16_t
, you only need to change one line.
source to share
This might be intended to force an unsigned int type for comparison, but unit16_t is not guaranteed to be the same size, and just specifying the cast state is sufficient.
Perhaps the status codes were listed in a previous version? It's int by default - but even then the casts don't make any sense imo.
Note that castings can even suppress out-of-size warnings. Your best bet would be to use an enum, or #define
just unsigned labels, eg. 0x00U
without type fixation.
source to share