views:

484

answers:

7

I'd like to see all the places in my code (C++) which disregard return value of a function. How can I do it - with gcc or static code analysis tool?

Bad code example:

int f(int z) {
    return z + (z*2) + z/3 + z*z + 23;
}


int main()
{
  int i = 7;
  f(i); ///// <<----- here I disregard the return value

  return 1;
}

Update:

  • it should work even if the function and its use are in different files
  • free static check tool
+4  A: 

Any static analysis code (e.g. PC-Lint) should be able to tell you that. For PC-Lint, I know that this is the case.

Xavier Nodet
+4  A: 

a static analyzer will do the work for you, but if your code base is more then trivial prepare to be overwhelmed ;-)

Alon
+7  A: 

As far as I'm aware there is no GCC option to give this warning. However, if you are interested in specific functions, you can tag them with an attribute:

int fn() __attribute__((warn_unused_result));

which would give a warning if the return value of fn() was not used. Caveat: I've never used this feature myself.

anon
+2  A: 

A static analyzer will be your best bet here. We use Coverity here, but there are free tools available that you can use as well.

If you need a quick-and-dirty solution and you have a Linux-style shell handy, you can try something like:

grep -rn "function_name" * | grep -v "="

That will find every line that references the specified function but does not contain an "=". You can get a lot of false positives (and potentially some false negatives) but if you don't have a static analyzer it's a decent place to start.

bta
+16  A: 

You want GCC's warn_unused_result attribute:

#define WARN_UNUSED __attribute__((warn_unused_result))

int WARN_UNUSED f(int z) {
    return z + (z*2) + z/3 + z*z + 23;
}

int main()
{
  int i = 7;
  f(i); ///// <<----- here i disregard the return value
  return 1;
}

Trying to compile this code produces:

$ gcc test.c
test.c: In function `main':
test.c:16: warning: ignoring return value of `f', declared with
attribute warn_unused_result

You can see this in use in the Linux kernel; they have a __must_check macro that does the same thing; looks like you need GCC 3.4 or greater for this to work. Then you will find that macro used in kernel header files:

unsigned long __must_check copy_to_user(void __user *to,
                                        const void *from, unsigned long n);
Eric Seppanen
i'm removing the answer acceptance, unfortunately it works in single file only.
Drakosha
Have you tried using it in header file?
el.pescado
Works fine in a header file for me. I put a function prototype with WARN_UNUSED in a new file lib.h and then included that from test.c and got the same warning. Also note this is how the Linux kernel does it.
Eric Seppanen
i'll recheck and post a code in case it doesn't work
Drakosha
sorry, i don't have time to recheck it, accepting the answer.
Drakosha
+3  A: 

You can use this handy template to do it at run-time.

Instead of returning an error code (e.g. HRESULT) you return a return_code<HRESULT>, which asserts if it goes out of scope without the value being read. It's not a static analysis tool, but it's useful none the less.

class return_value
{
public:
  explicit return_value(T value)
    :value(value), checked(false)
  {
  }

  return_value(const return_value& other)
    :value(other.value), checked(other.checked)
  {
    other.checked = true;
  }

  return_value& operator=(const return_value& other)
  {
    if( this != &other ) 
    {
      assert(checked);
      value = other.value;
      checked = other.checked;
      other.checked = true;
    }
  }

  ~return_value(const return_value& other)
  {
    assert(checked);
  }

  T get_value()const {
    checked = true;
    return value;
  }

private:
  mutable bool checked;
  T value;
};
Joe Gauterin
+1 - that's a neat idea
Bill
It's a nice idea. Unfortunately it violates the "destructors mustn't throw" principle: http://www.parashift.com/c++-faq-lite/exceptions.html#faq-17.3, but maybe that'd be acceptable for a "check all returns" testing mode (there's probably no need for the check in a final release build).
timday
Good point - I've changed it to assert instead.
Joe Gauterin
+2  A: 

The classic 'lint' program used to be very voluble about functions that returned a value that was ignored. The trouble was, many of those warnings were unwanted - leading to excessive noise in the lint output (it was picking up bits of fluff that you wanted it to ignore). That's probably why GCC doesn't have a standard warning for it.

The other issue - the flip side - is "how do you suppress the warning when you know you are ignoring the result but really don't care". The classic scenario for that is:

if (signal(SIGHUP, SIG_IGN) != SIG_IGN)
    signal(SIGHUP, sighandler);

You care about the first result from signal(); you know that the second will be SIG_IGN (since you just set it to that). To get away from the warnings, I sometimes use some variant on:

if ((old = signal(SIGHUP, SIG_IGN)) != SIG_IGN)
    old = signal(SIGHUP, sighandler);

This assigns to old both times. You can follow that with 'assert(old == SIG_IGN)'.

Jonathan Leffler
Actually, the typical way of saying "no, I really don't care about the return value" is with a cast to `void`, e.g. `(void)printf("Hello, world!\n");` Even at the highest warning level, no warning will be emitted for this with the explicit cast to void.
Adam Rosenfield
Yes, the '`(void)`' cast works...it looks ugly in the code, though. And I've got to look after code which replaces that ugly-gram with '`VOID`' instead; it maps to '`(void)`' cast with Standard C compilers (all C compilers these days), but originated in the days before they were standard and then did things with assigning the result to a file static variable ('`#define VOID _void_ =`' or thereabouts). Aaah; the memories, and the hoops that had to be jumped through.
Jonathan Leffler