1

I am trying to get rid of violation of rule 15.5 from my code.

Sample code:

#define RETURN_VAL(num) {return (2 * num);} 

static int32_t
func(int32_t n1, int32_t n2, int32_t n3)
{  
    if (n1 == 1) {                
        RETURN_VAL(1);
    }
    if (n2 == 2) {
        RETURN_VAL(2);                  
    }
    if (n3 == 3) {
        RETURN_VAL(3);             
    }

    return 0;
}

Since the MACRO(having return value) is using in multiple places, result in violation of Rule 15.5.

Is there anyway to fix this keeping as MACRO itself.

Cœur
  • 37,241
  • 25
  • 195
  • 267
Amardeep
  • 89
  • 3
  • 8
  • 1
    Change it all accordingly so it becomes `return MACRO(num);` No way to have your cake and eat it too. Keeping the macro itself is also ill-advised. But that's another matter. – StoryTeller - Unslander Monica Jan 02 '18 at 08:32
  • 3
    Note that having macros that hide control flow are considered a bad practice. You really should rewrite the code to remove it. – user694733 Jan 02 '18 at 08:32
  • 3
    What's the point of the original macro, besides confusing the reader of your code? – Matteo Italia Jan 02 '18 at 08:34
  • 1
    If you are developing code that requires MISRA that snippet should not pass review anyway (at least i wouldn't let it pass). You are hiding code flow. Your macro does not state that it will modify the return value. You have multiple returns (even removing the macro and having them in the code is still a violation of MISRA). – RedX Jan 02 '18 at 08:36
  • You should have a look at https://stackoverflow.com/questions/42575331/best-practice-for-compute-the-function-return-value/. – Melebius Jan 02 '18 at 08:40
  • What do you mean by "keeping MACRO itself"? You keep the macro? You keep the invocations? – Antti Haapala -- Слава Україні Jan 02 '18 at 08:47

2 Answers2

2

Regarding using multiple return statements in a function, it is perfectly fine if it leads to more readable code. Rule 15.5 is a questionable one, with a questionable rationale, see this.

That being said, the main problem here is the use of pointless function-like macros, which violates Directive 4.9 of MISRA-C. Overall, the function interface is strange, do you really need to use 3 named parameters? A better alternative:

static int32_t func(int32_t n[N])
{  
    for(int32_t i=0; i<N; i++)
    {
      if(n[i] == i+1)
      {
        return 2 * (i+1);
      }
    }
    return 0;
}

If you must preserve the icky API for some reason, then use a wrapper:

inline static int32_t func_wrapper (int32_t n1, int32_t n2, int32_t n3)
{
  return func( (int32_t[3]){n1, n2, n3} );
}
Lundin
  • 195,001
  • 40
  • 254
  • 396
1

You can modify the code above to remove multiple returns using an integer retval

If you absolutely have to use a Macro before returning use one as shown below. Do not hide the return statement with a macro. It will not affect any MISRA analysis.

#define  RET(num) (2 * num)

static int32_t
func(int32_t n1, int32_t n2, int32_t n3)
{  
    int32_t retval = 0;
    if (n1 == 1) {                
        retval = RET(1);
    }
    else if (n2 == 2) {
        retval = RET(2);
    }
    else if (n3 == 3) {
        retval = RET(3);             
    }

    return retval;
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Rishikesh Raje
  • 8,556
  • 2
  • 16
  • 31