FP due to ignoredReturnValue and usage

[Steve]

I was going through and fixing some false positives generated because of the wxWidgets.cfg. I ran across a few that might require a bit more thought.

We have several that I would consider FPs in our code base because of wxFileConfig::Read. The signature is:
bool wxConfigBase::Read(const wxString &key, wxString *str) const

Naturally because it's a const function with a return value you would probably want to use the return value. However, most (if not all) of our code is something like this:
wxString value("default"); cfg.Read("key", &value); DoSomething(value);

So essentially we don't care whether the return value was true or false because the in/out parameter is used as a return value and we're going to do the same thing regardless. The key is optional. So I guess what I'm getting at is should cppcheck treat an in/out value as a return value (assuming it was initialized)? Should we just be explicit that we don't care about the return value with a (void) in front or something?

However, there was also a case of this:
int majorH, minorH; wxGetOsVersion(&majorH, &minorH); //Return value of function wxGetOsVersion() is not used.

I would still want some sort of error for this. Maybe this should be an inconclusive uninitVar error. It could try to be smart about it by seeing if the return value is checked when passing uninitialized values to a function or maybe this is just another cfg attribute that output variables are only set on success or something.

There's also the issue of another overloaded signature:
const wxString wxConfigBase::Read(const wxString &key, const wxString &defaultVal) const

That one you certainly would want to use the return value, but it doesn't appear I can have both of those defined in the same cfg. It seems like the later one wins.

Then on on the write side of wxFileConfig we never check the return value so it complains about that too. I'm somewhat questioning if it should be marked that way in the cfg. Yes technically there's a return value that indicates that it was successful or not. However, this isn't like file writing it's just building up internal data structures that it will write out to the file when you call Flush(). There's only one path where it would actually return false and that's a very specific case where the key starts with ! and that will also show an error dialog or stderr output (depending on application). So again we wouldn't do anything based on the return value, but we could certainly explicitly state that with (void) if that's the recommendation.

[Daniel Marjamäki]

So essentially we don't care whether the return value was true or false because the in/out parameter is used as a return value and we're going to do the same thing regardless. The key is optional. So I guess what I'm getting at is should cppcheck treat an in/out value as a return value (assuming it was initialized)? Should we just be explicit that we don't care about the return value with a (void) in front or something?

The Cppcheck - philosophy would be to not warn about that. It seems to me the configuration should be changed.

I have the feeling that some addon will report a warning if a non-void return value is not used. So that addon will warn thanks to the <returnValue type="bool"/> configuration. The <use-retval/> configuration is wrong.

I would still want some sort of error for this. Maybe this should be an inconclusive uninitVar error. It could try to be smart about it by seeing if the return value is checked when passing uninitialized values to a function or maybe this is just another cfg attribute that output variables are only set on success or something.

Then I believe the configuration for the argument should say <not-uninit/>

That one you certainly would want to use the return value, but it doesn't appear I can have both of those defined in the same cfg. It seems like the later one wins.

No that is a major problem!! It is not by intention I wish we can fix this someday.

[Daniel Marjamäki]

Then I believe the configuration for the argument should say <not-uninit></not-uninit>

hmm I think I misunderstood.. you can't currently configure Cppcheck so it can be checked properly but I would not be against that it is possible to configure it properly if we can design something relatively simple..