issue #308, Resolve cpp warning in os dir.#310
issue #308, Resolve cpp warning in os dir.#310skliper merged 1 commit intonasa:merge-20191230from avan989:cppcheck_os
Conversation
jphickey
left a comment
jphickey
left a comment
There was a problem hiding this comment.
Conditionally approved, but I would like to confirm if there isn't some other way to avoid the cppcheck warning other than the inline suppression (e.g. by telling it that OS_MAX_MODULES > 0 or just giving it the path to osconfig.h)?
| */ | ||
| typedef struct | ||
| { | ||
| /* cppcheck-suppress unusedStructMember */ |
There was a problem hiding this comment.
This seems suspicious because the dl_handle is absolutely used, but the code is conditional upon OS_MAX_MODULES > 0 in the config.
Maybe cppcheck is complaining because it doesn't know what OS_MAX_MODULES is set to, therefore assumes that the conditional is false?
There was a problem hiding this comment.
Right. Cppcheck is checking both conditions. I prefer this specific suppression vs passing in OS_MAX_MODULES to cppcheck, in that I don't really care about this specific warning but if future ones show up for that condition being false I want to know.
Alternatively, if the condition really can never happen, then it probably shouldn't be there (and add a verify that OS_MAX_MODULES > 0).
There was a problem hiding this comment.
The condition of OS_MAX_MODULES being 0 can happen. But in that case the structure isn't instantiated at all. cppcheck just does not know that.
I think a better long-term solution would be to work this into the cmake-based osconfig options (to replace the preprocessor-based logic). This will get rid of these types of #if conditionals in here and probably clear up this warning too.
skliper
added
the
CCB:Approved
|
CCB 20191218 - Reviewed and approved for IC |
4 participants
Describe the contribution
Resolve cppcheck for os dir. Possible cppcheck bug for structure member - suppress.
Testing performed
Steps taken to test the contribution:
System(s) tested on:
Contributor Info
Anh Van, NASA Goddard