[Ecls-list] Coverity Scan defect report for ECL

[Arto Bendiken]

Good afternoon,

As noted to Matt in the other thread, I've registered the ECL project
with the Coverity Scan [1] static analysis cloud service—offered free
of charge to open-source projects—and have had them crunch through the
C portions of the code base.

Submitting a first build of HEAD to them yesterday, Coverity reported
639 defects [2]. I've triaged a number of the defects and thus far
eliminated or mitigated 23 of them through various means; see the
recent commits to HEAD. Note that in commit messages, CID means
Coverity ID—the identifier for a defect report in Coverity Scan.

The breakdown by category for the remaining 616 outstanding defect
reports is as follows:

• Control flow issues: 249
• API usage errors: 233
• Memory - illegal accesses: 83
• Uninitialized variables: 20
• Integer handling issues: 8
• Error handling issues: 8
• Code maintainability issues: 7
• Memory - corruptions: 3
• Insecure data handling: 1
• Null pointer dereferences: 1
• Incorrect expression: 1
• Security best practices violations: 1
• Resource leaks: 1

That perhaps looks more intimidating than is the reality. Many of
these are duplicate issues, i.e., the same class of report applies to
multiple locations in the code base. This is particularly the case for
defects reported in the generated C code under the build/ directory.

Also, a minority of the defect reports are false positives. To reduce
such cases, I've defined a so-called modeling file for Coverity Scan.
It's committed at src/c/coverity/model.c [3]. It could use some more
work to describe more of the ECL functions tagged with the
'ecl_attr_noreturn' attribute, which would reduce false positives
about missing return and break statements.

While I've tackled some of the lowest-hanging fruit, many of the
remaining defects are rather subtle, involving excessively convoluted
control flow. Coverity finds them because they trace execution through
all possible branches in the control flow graph, annotating the
problematic sequence of branches in the defect reports.

In one such case I partially addressed [4], I felt it risky to do more
than add a sanity-check assertion that will catch the uninitialized
pointer read if control should indeed ever flow in the unlikely(?)
sequence of branches identified by Coverity. Perhaps someone with more
experience with that piece of code can do better.

To view the defect reports and help quash these bugs, you can register
a free account with the Coverity Scan service and then request access
to the project there [2]. Unless there any objections, I would just go
ahead and give out access to anyone who asks to contribute to fixing
these issues. (In case there should be valid security concerns for
more restricted access, please do let me know soonest.)

Cheers,
Arto

[1] https://scan.coverity.com
[2] https://scan.coverity.com/projects/3235
[3] https://sourceforge.net/p/ecls/ecl/ci/03e4303ff8d76325036f4e603687a6d3dc36001b
[4] https://sourceforge.net/p/ecls/ecl/ci/633c3a5f63a602ce18c54bfe88922d8644cbe619

-- 
Arto Bendiken | @bendiken | http://ar.to