[Automated review] Coverity noise is too much
Categories
(Developer Infrastructure :: Source Code Analysis, defect)
Tracking
(Not tracked)
People
(Reporter: emilio, Assigned: andi)
References
(Blocks 1 open bug)
Details
Phabricator URL: https://phabricator.services.mozilla.com/D30830 (but many others too, really)
-
Coverity warns for a lot of code that I haven't touched in my patch.
-
Coverity doesn't seem to understand MOZ_ASSERT / NS_ASSERTIONs that assert that a pointer is non-null, causing a lot of noise. For example, two of the messages in that CL is about dereferencing null pointers that are asserted against on the line below. Those pointers being null would be a logic error.
Reporter | ||
Comment 1•6 years ago
|
||
Also:
- Coverity bugs me each time I update the CL, even if the same warning was emitted before.
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Comment 3•6 years ago
|
||
FWIW, Coverity also doesn't understand basic template, but still attempts to make apparently wrong suggestion:
See https://phabricator.services.mozilla.com/D29992.
For a simple template with only a few lines, where a parameter is used to store output (*aRes = ...
):
template <class... Tags>
bool ResolveAll(const SVGElement* aElement,
details::AlwaysFloat<Tags>*... aRes) {
if (nsIFrame const* f = aElement->GetPrimaryFrame()) {
using dummy = int[];
(void)dummy{0, (*aRes = ResolveWith<Tags>(*f->Style(), aElement), 0)...};
return true;
}
return false;
}
Coverity tells me to mark it as const
:
Warning: Pointer parameter 'aRes' can be pointer to const [clang-tidy: readability-non-const-parameter]
Checker reliability (false positive risk) is high.
This warning appears over and over again. It's really annoying. I think we should at least have the option to manually mark a Coverity suggestion as wrong, so that it won't appear again.
Assignee | ||
Comment 4•6 years ago
|
||
@violet.bugrepor the issue that you report is not related with Coverity
analyser, but it's a clang-tidy
checker. Besides the checker result you have it's reliability, do you think we should alter the reliability of this checker?
Comment 5•6 years ago
|
||
Besides the checker result you have it's reliability, do you think we should alter the reliability of this checker?
I'm not sure whether the checker happens to have trouble with this particular piece of code, or it is incapable of analyzing template in general. So I don't have an opinion on that.
I think the important thing is that user should be able to silence a particular warning, so that it doesn't appear over and over again, after every change. If that could be done, then some false positive doesn't matter.
Updated•6 years ago
|
Assignee | ||
Comment 6•6 years ago
|
||
Closing this as it has been addressed in other bugs and It was fixed since last week, and the fix is present in the latest release of releneg-services
.
Assignee | ||
Updated•6 years ago
|
Updated•2 years ago
|
Description
•