On 3/15/2017 2:31 PM, Jeffrey Walton wrote:
Does Coverity have a way in which in the code it can be marked as OK? (I'd
expect some '#pragma CoverityIgnore(bits)' or the like ought to be
available.)
Yes. You have to provide a modeling file. Also see the Coverity Scan
FAQ entry "what is a model" at https://scan.coverity.com/faq.
A model is for odd cases like where you have your
own memory allocators or synchronization primitives.
I think this is a case for what they call annotations.
I have never written one of these but I think we would
have to add a comment something like this ahead of the
call:
/* coverity[uninit_use_in_call] */
I will try adding that notation but we clearly need
some guidelines as a project.
Other projects use them, like Python. See, for example,
https://docs.python.org/devguide/coverity.html.
I agree with trying to get rid of the message, but it is worth
bloat to do it? (It will add instructions to either initialize bits to 0 or
add the else.)
If I am parsing things correctly, it seems like the bloat is going the
other way: if the code is not needed, then remove it. It will avoid
findings like these, and speed up the compile.
I would rather mark something in the code as a false
positive than add code because the tool is not smart enough to know--so we
might differ in philosophy there.
Perhaps a better strategy would be to initialize all variables, and
then allow the optimizer to remove the unneeded writes. It will ensure
a program is in a good state, and avoid findings like these.
I'm a middle of the road guy. I add initialization in cases
where there are paths where it is used and doesn't otherwise
get set. I wouldn't automatically initialize everything.
In this case, "bits" is actually used on the RHS multiple
times in ULtod() so it bothers me that it has an undefined
value. That means the output of ULtod() is undefined in
this case.
Another strategy is to do nothing. In this case, the same findings
will waste multiple developer's time, and generate additional mailing
list messages.
Agreed.
I like dark and silent cockpits, so I don't want tools generating
findings, and I don't want mailing list messages. I would squash it
once and for all and avoid all future problems. But that's just me,
and I understand the Newlib project may have a different outlook on
things.
+1
There are still 60 others issues. We should do our best to squash
them permanently. IMO marking them with Coverity specific annotation
just means that another static analyzer may find the same issue
in the future. The annotation won't help.
Jeff
--joel