I am writing two different functions with two different parameter data types. Additionally, both of the functions have different return types. The first function Function_One_u4 has two parameters of type uint8: signal_one_u4 and signal_two_u4. On the other hand, function Function_Two_u16 has similiar pattern, but two signals are of type uint16: signal_one_u16 and signal_two_u16. Thus, first and second functions have return type uint8 and uint16, respectively. Additionally, both functions return different ERROR values in the default statements.
uint8 Function_One_u4(const uint8 mode_u2,
const uint8 signal_one_u4,
const uint8 signal_two_u4)
{
switch(mode_u2)
{
case NOT_ACTIVE_U2:
{
return signal_two_u4;
}
case ACTIVE_U2:
{
return signal_one_u4;
}
case ERROR_U2:
{
return signal_one_u4;
}
case NOT_AVLB_U2:
{
return signal_two_u4;
}
default:
{
return ERROR_U4; /* Return value of 15 */
}
}
}
uint16 Function_Two_u16(const uint8 mode_u2,
const uint8 signal_one_u16,
const uint8 signal_two_u16)
{
switch(mode_u2)
{
case NOT_ACTIVE_U2:
{
return signal_two_u16;
}
case ACTIVE_U2:
{
return signal_one_u16;
}
case ERROR_U2:
{
return signal_one_u16;
}
case NOT_AVLB_U2:
{
return signal_two_u16;
}
default:
{
return ERROR_U16; /* Return value of 65535 */
}
}
}
void main(void)
{
uint8 ret_val_u4 = Function_One_u4();
uint16 ret_val_u16 = Function_Two_u16();
}
You can notice that functions pretty much have the same logic - based on the parameter uint8 mode_u2, they return either first or second signal. Therefore, it makes sense to make a generic function with the help of templates. The generic function would avoid duplicating the switch case code:
<T> Generic_Function_<T> (const uint8 mode_u2,
const <T> signal_one,
const <T> signal_two,
const <T> error)
{
switch(mode_u2)
{
case NOT_ACTIVE_<T>:
{
return signal_two;
}
case ACTIVE_<T>:
{
return signal_one;
}
case ERROR_<T>:
{
return signal_one;
}
case NOT_AVLB_<T>:
{
return signal_two;
}
default:
{
return error;
}
}
}
uint8 Function_One_u4(const uint8 mode_u2,
const uint8 signal_one_u4,
const uint8 signal_two_u4)
{
Generic_Function_<T>(mode_u2, signal_one_u4, signal_two_u4);
}
uint16 Function_Two_u16(const uint8 mode_u2,
const uint8 signal_one_u16,
const uint8 signal_two_u16)
{
Generic_Function_<T>(mode_u2, signal_one_u16, signal_two_u16);
}
However, C language does not support templates. I have found online that one can create C templates using preprocessor macros. But I also read that using macros for functions should be avoided as they increase a chance of introducing errors in your code. I am writing a safety-critical MISRA Software in C language, so unexpected errors sneaking in the code would not be something very nice :).
Is there some other suggestion on how to handle the code duplication? Thank You in advance!
Generally, programmers have a tendency to get too extreme in their attempts to avoid code repetition. It should be avoided when possible, sure, but not at any cost. In the context of safety-critical systems, generic programming is to be avoided:
First, you must at least in theory be able to prove that all execution paths are actually executed - code coverage. Generic programming can add false safety here, if you merge 2 functions into one then that function does get executed, but only for one of the use-cases.
Second, safety-critical system must be 100% deterministic with no unknown parameters. So traditionally generic programming doesn't make a lot of sense to begin with. You should ideally be able to trace every requirement in the specification to a certain chunk of code and then design a test for that code. This part gets a whole lot easier with specialized functions instead of generic ones.
There are tricks with macros or C11 _Generic (banned by MISRA-C) that could be used here, but I think this specific case could be simplified without making things too complicated. Why not simply add an additional parameter for "U type"? I see no obvious reason why the functions can't be rewritten like this:
uint16_t function (uint8_t mode
uint8_t singal_one,
uint8_t signal_two
utype_t utype)
{
uint16_t result;
switch(mode)
{
case NOT_ACTIVE: { result = signal_two; break; }
case ACTIVE: { result = signal_one; break; }
case ERROR: { result = signal_one; break; }
case NOT_AVLB: { result = signal_two; break; }
default:
{
if(utype==U2)
{
result = (uint16_t) ERROR_U2;
}
else
{
result = ERROR_U4;
}
}
}
return result;
}
I removed the const from the parameters since that part is just subjective coding style unrelated to safety.
Notes from a MISRA-C context:
uint8_t to uint16_t is fine in MISRA-C, they are of the same essential type category. You just have to dodge the silly rule about composite expressions getting assigned to a wider type, hence the convoluted default above, which assumes ERROR_U2 is 8 bits but ERROR_U4 is 16 bits.I didn't run this code in static analysis but I think it's MISRA-C compliant.
Just for reference, the macro alternative, which questionable both in general and from a MISRA-C point-of-view, would go like this:
#define function(n) uint16_t function_##n (uint8_t mode_u2, ...
...
default: { result = ERROR_##n;
And then you can have this macro generating 2 different functions with function(U2) and function(U4) and let the macro expand the contents of the function to different enums etc. Doesn't solve the return type though, so you might have to do as in my first example still.