tricky situations with cppcheck

Question

I am facing a situation where i need cppchecks to pass but it gets tricky sometimes. What do you generally do in such circumstances ? For example.

#include<iostream>
using namespace std;
void fun1();
int fun2();
int main()
{
        fun1();
}

void fun1()
{
        int retVal;
        if (-1 == (retVal = fun2()))
        {
                cout <<"Failure. fun2 returned a -1"<< endl;
        }
}

int fun2()
{
        return -1;
}

We usually see code such as the above. cppcheck for the above file would give an output as below -

cppcheck --suppress=redundantAssignment --enable='warning,style,performance,portability' --inline-suppr --language='c++' retval_neverused.cpp Checking retval_neverused.cpp... [retval_neverused.cpp:13]: (style) Variable 'retVal' is assigned a value that is never used.

I don't want to add some dummy line printing retVal just for the sake of cppcheck. Infact it can be a situation where I throw an exception and I don't want the exception to have something trivial as the value of retVal in it.

I don't get the Q. You don't use `retVal` after you assign a value so the diagnosis is correct. Right now you could rewrite the code as `void fun1() { if (-1 == fun2()) { cout <<"Failure. fun2 returned a -1"<< endl; } }` — NathanOliver, Jan 03 '19 at 17:12

score 6 · Answer 1 · answered Jan 03 '19 at 17:13

CppCheck is kinda right though. You don't need retVal at all. just check the return value of fun2 directly: if( -1 == fun2() )

As an aside, assigning variables inside conditional expressions is really bad practice. It makes it a lot harder to catch typos where you meant to type == but actually typed =.

Thomas Matthews · Answer 2 · 2019-01-03T21:52:53.723

4

You could rewrite as:

const int retval = fun2();
if (retval == -1)

This technique is, IMHO, easier to debug because you can see, with a debugger, the value returned from fun2 before the if statement is executed.

Debugging with the function call in the if expression is a little more complicated to see the return value from the function.

edited Jan 03 '19 at 21:52

answered Jan 03 '19 at 19:32

Thomas Matthews

56,849
17
98
154

David Schwartz · Accepted Answer · 2019-01-04T18:35:50.693

One common way is with something like this:

#define OK_UNUSED(x) (void)x


void fun1()
{
        int retVal;
        if (-1 == (retVal = fun2()))
        {
                OK_UNUSED (retVal);
                cout <<"Failure. fun2 returned a -1"<< endl;
        }
}

This indicates to humans that retVal is intentionally unused and makes CppCheck think it's used, suppressing the warning.

Note that this macro should not be used if evaluating its parameter has consequences. In that case, you need something fancier like:

#define OK_UNUSED(x) if(false && (x)) ; else (void) 0

tricky situations with cppcheck

3 Answers3