This completes my batch.
A summary and my notes can be found at
http://www.users.on.net/~kenj/pcp/coverity-1.html
I'd appreciate feedback from other eyes and brains for the ones marked
"Needs review", namely #30, #32, #325, #327, #337, #345, #349 and #350.
Overall, I'd have to say the issues identified by Coverity's static
analysis in this body of code are corner cases, usually on the unlikely
error paths, where the effects of the "issue" are very small ...
something akin to when the nuclear war starts, make sure you wash the
dishes before heading for the shelter, whereas we've tended to assume
that washing the dishes will not make a significant difference.
Never the less, an interesting exercise.
Changes committed to git://oss.sgi.com/kenj/pcp.git dev
src/libpcp/src/connectlocal.c | 1
src/libpcp/src/context.c | 1
src/libpcp/src/derive.c | 59 +++++++++++++++++++++++-------------
src/libpcp/src/derive.h | 1
src/libpcp/src/derive_fetch.c | 4 ++
src/libpcp/src/fetch.c | 4 --
src/libpcp/src/fetchlocal.c | 11 +++++-
src/libpcp/src/interp.c | 29 +++++++++++++-----
src/libpcp/src/logmeta.c | 13 ++++++--
src/libpcp/src/logportmap.c | 18 ++++++-----
src/libpcp/src/logutil.c | 46 ++++++++++++++++++++++++----
src/libpcp/src/optfetch.c | 2 +
src/libpcp/src/p_pmns.c | 22 ++++++++-----
src/libpcp/src/p_result.c | 11 ------
src/libpcp/src/pmns.c | 67 +++++++++++++++++++++++++-----------------
src/libpcp/src/profile.c | 4 +-
src/libpcp/src/spec.c | 2 +
src/libpcp/src/units.c | 2 -
src/libpcp/src/util.c | 64 +++++++++++++++++++---------------------
src/pmdas/logger/event.c | 2 +
20 files changed, 231 insertions(+), 132 deletions(-)
commit ceb12dd91a882563b4e4ec5b15cbc4254ce27dee
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Sat Jan 28 16:00:14 2012 +1100
libpcp/logutil.c - use_after_free
Original Coverity case #345
http://people.redhat.com/mgoodwin/pcp-cov/1/345logutil.c.html#error
Added another check to ensure FILE *f from _logpeek() is closed.
This does not address the original Coverity issue, which I believe
shows incorrect analysis.
commit accaf3fe8ef4fcd44d4f4260319c3ace14e5a561
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Sat Jan 28 14:34:12 2012 +1100
libpcp/pmns.c - uninit
Original Coverity case #298
http://people.redhat.com/mgoodwin/pcp-cov/1/298pmns.c.html#error
A timeout is also an IPC error ... when removes the Coverity issue
about numnames not being set in rare cases.
commit 4ba8377649e2521f18616b49ec8d90069c1eed22
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Sat Jan 28 14:28:27 2012 +1100
libpcp/p_pmns.c - reverse_inull
Original Coverity case #256
http://people.redhat.com/mgoodwin/pcp-cov/1/256p_pmns.c.html#error
Remove pointless test for namelist != NULL.
commit 6b8534a141b72a384da0c410675d3c2b9a0071a3
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Sat Jan 28 14:23:43 2012 +1100
libpcp/logmeta.c - resource_leak
Original Coverity case #247
http://people.redhat.com/mgoodwin/pcp-cov/1/247logmeta.c.html#error
Added some free(dp) calls on the error branches.
commit dccd6bee204338bf6d6a2b8849926a7efb982ef9
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Sat Jan 28 06:34:56 2012 +1100
libpcp/fetchlocal.c - resource_leak
Original Coverity case #230
http://people.redhat.com/mgoodwin/pcp-cov/1/230fetchlocal.c.html#error
Better cleanup of malloc'd regions on error branch.
commit 9c0dd0e21dc07e93f7b00f9d0f3b27834828792c
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Sat Jan 28 06:26:38 2012 +1100
libpcp/interp.c - resource_leak
Original Coverity case #189
http://people.redhat.com/mgoodwin/pcp-cov/1/189interp.c.html#error
Other than the "numinst==0" corner case, there is no functional
change here ... rework the guards around the free() calls in the
hope that this will convince Coverity that there is no issue.
commit 55cbb6ce25b846aa8a4ccfbf2d492b7e6a7f1b54
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Fri Jan 27 22:24:36 2012 +1100
libpcp/logmeta.c - resource_leak
Original Coverity cases #181 and #204
http://people.redhat.com/mgoodwin/pcp-cov/1/181logmeta.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/204logmeta.c.html#error
Added calls to free() to release local allocations on error
branches.
commit c978eae62d43e7ca61eab56c837c38b0febf97db
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Fri Jan 27 21:40:54 2012 +1100
libpcp/logportmap.c - resource_leak
Original Coverity case #164
http://people.redhat.com/mgoodwin/pcp-cov/1/164logportmap.c.html#error
Add free(laddrs) on error branch.
commit 1192405b51d3c1bc793b4a100719f01a81af9d23
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Fri Jan 27 21:30:25 2012 +1100
libpcp/connectlocal.c - resource_leak
Original Coverity case #163
http://people.redhat.com/mgoodwin/pcp-cov/1/163connectlocal.c.html#error
Add free(config) on error branch.
commit f5132da3cf24c24c9a6dded81d782aa0f0d45f90
Merge: ee5f031 b15b6ef
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Fri Jan 27 15:00:47 2012 +1100
Merge branch 'oops' into dev
commit b15b6ef72f829a5466b169bd6486001b542f61ee
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Fri Jan 27 13:24:45 2012 +1100
libpcp/fetch.c - resource_leak
Original Coverity case #161
http://people.redhat.com/mgoodwin/pcp-cov/1/161fetch.c.html#error
No real problem here (newlist is not set in __dmprefetch() if newcnt
<= numpmid on return) ... cosmetic code rearrangement to try and stop
Coverity complaining.
commit ee5f0319c37bab74273403f26915c7cec8b2ac31
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Fri Jan 27 12:16:10 2012 +1100
libpcp/context.c - resource_leak
Original Coverity case #155
http://people.redhat.com/mgoodwin/pcp-cov/1/155context.c.html#error
Call free(errmsg) on the error branch after reporting the error.
commit 15fbdcbae3dd737d54f4c532af7e4ec7cda23b54
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Fri Jan 27 07:10:13 2012 +1100
libpcp/pmns.c - resource_leak
Original Coverity cases #153, #156, #165, #166, #173, #178, #233,
#234 and #242.
http://people.redhat.com/mgoodwin/pcp-cov/1/153pmns.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/156pmns.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/165pmns.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/166pmns.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/173pmns.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/178pmns.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/233pmns.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/234pmns.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/242pmns.c.html#error
On several error branches, add calls to free() to release storage
that has been allocated but that will not be used as a result of
the error condition ... all very low probability of being seen
and low impact.
commit 51606026be68c56b77d7f8835663e1021fad92e7
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Fri Jan 27 06:50:50 2012 +1100
libpcp/spec.c - resource_leak
Original Coverity case #147.
http://people.redhat.com/mgoodwin/pcp-cov/1/147spec.c.html#error
Conditionally call free(host) on error branch.
commit 9f601a5c6a3fa78443bba6578f2c705b61bd2894
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Fri Jan 27 06:25:32 2012 +1100
libpcp/interp.c - null_returns
Original Coverity cases #128, #129, #132 and #133.
http://people.redhat.com/mgoodwin/pcp-cov/1/128interp.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/129interp.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/132interp.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/133interp.c.html#error
Add assert() to ensure result from __pmHashSearch() is not NULL
in the places where we assume this to be the case.
commit 62a0c2e085c44039de144482e2f5b141399d949f
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Fri Jan 27 06:20:00 2012 +1100
libpcp/units.c - no_effect
Original Coverity case #124.
http://people.redhat.com/mgoodwin/pcp-cov/1/124units.c.html#error
commit d89bda21816ffaf822a329d97735f104a67b97bd
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Thu Jan 26 22:54:19 2012 +1100
libpcp/logutil.c - use_after_free
Original Coverity case #342.
http://people.redhat.com/mgoodwin/pcp-cov/1/342logutil.c.html#error
__pmlogwriteLabel() changed to not call fclose() on the error branch ...
failure is reported via pmprintf() in __pmlogwriteLabel() and we tend
to assume __pmlogwriteLabel() just works, so there is little or no error
handling in the caller code.
commit 6f0b83e4895eac1d7dda65b47c491bcff389837c
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Thu Jan 26 22:39:44 2012 +1100
libpcp/logutil.c - resource_leak
Original Coverity case #217.
http://people.redhat.com/mgoodwin/pcp-cov/1/217logutil.c.html#error
Better capture of preconditions for pmFreeResult(rp) call on return
path.
commit afd497d87027cb502f9a610920718d6010e365b9
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Thu Jan 26 22:19:38 2012 +1100
libpcp/logutil.c - resource_leak
Original Coverity case #174.
http://people.redhat.com/mgoodwin/pcp-cov/1/174logutil.c.html#error
Add fclose(f) on unlikely error branch.
commit 987c91de0a7a5de2d3087e0c9d7832f7e142a10c
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Thu Jan 26 22:02:20 2012 +1100
libpcp/util.c - negative_returns
Original Coverity case #109.
http://people.redhat.com/mgoodwin/pcp-cov/1/109util.c.html#error
Check dup() fails case.
commit 0276fb926dc94f9173284e78d8cb38a72b8ab23c
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Thu Jan 26 21:34:25 2012 +1100
libpcp/derive.c - resource_leak
Original Coverity case #235.
http://people.redhat.com/mgoodwin/pcp-cov/1/235derive.c.html#error
fclose(fp) added.
commit a3393e515b641dcbcf2556bc96e10dc71a1df1c6
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Thu Jan 26 21:14:28 2012 +1100
libpcp/derive.c - resource_leak
Original Coverity case #216.
http://people.redhat.com/mgoodwin/pcp-cov/1/216derive.c.html#error
Added assert() and explanatory comment to emphasize that P_INIT code is
only executed once per expression.
This commit also fixes issues #221 and #229.
commit f3717b333ce05b402abd0765098d66d3f7f0a531
Merge: 52fd399 0a0df22
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Thu Jan 26 16:20:03 2012 +1100
Merge branch 'dev' of git://oss.sgi.com/markgw/pcp/pcp into dev
commit 52fd399c829b656dca7b3d2d59279741deedb334
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Thu Jan 26 16:11:04 2012 +1100
libpcp/derive.c - resource_leak
Original Coverity cases #172 & #196
http://people.redhat.com/mgoodwin/pcp-cov/1/172derive.c.html#error
http://people.redhat.com/mgoodwin/pcp-cov/1/196derive.c.html#error
Added explicit assert().
Also redo the fix for Original Coverity case #307.
http://people.redhat.com/mgoodwin/pcp-cov/1/307derive.c.html#error
which was botched in commit 2b3964bbcb0c17b6c6f9374578c3458061644826.
commit 0a0df221de0ae11c28d57c92d338126b3a50624f
Author: Mark Goodwin <mgoodwin@xxxxxxxxxx>
Date: Wed Jan 25 11:21:25 2012 +1100
Defensive change to check the file descriptor before the read,
rather than relying on EBADF error handling.
modified: src/pmdas/logger/event.c
commit 7ac8602147ae21359977b3cfb03b1dd8d93e51fd
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Wed Jan 25 06:47:32 2012 +1100
libpcp/derive.c - resource_leak
Original Coverity case #212.
http://people.redhat.com/mgoodwin/pcp-cov/1/212derive.c.html#error
Add one more free_expr(expr) before return NULL.
commit 009518a5c4e80c449fa7acade054e0aa59cdeb78
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Wed Jan 25 06:31:29 2012 +1100
libpcp/derive.c - unused_value
Original Coverity case #316.
http://people.redhat.com/mgoodwin/pcp-cov/1/316derive.c.html#error
Report string context where (unexpected) parser error occurs.
commit 2b3964bbcb0c17b6c6f9374578c3458061644826
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Wed Jan 25 06:22:58 2012 +1100
libpcp/derive.c - unreachable
Original Coverity case #307.
http://people.redhat.com/mgoodwin/pcp-cov/1/307derive.c.html#error
Set eof, then return ... duh!
commit 00e76cdccdd3d3eefb4d0a64ab14f3b4f5e8e2c3
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Tue Jan 24 14:51:56 2012 +1100
libpcp/interp.c, libpcp/logutil.c & libpcp/pmns.c - negative ftell() returns
Original Coverity case #105.
http://people.redhat.com/mgoodwin/pcp-cov/1/105interp.c.html#error
This commit also reworks the changes for Coverity cases #91 and #98.
In all the places where we use the results from ftell(), add an assert
to ensure the result is >= 0 ... the error cases should not happen and
are only expected by Coverity ... 8^(>
commit ece0188d94541906e5746545481ae5b082a3e560
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Tue Jan 24 14:30:21 2012 +1100
libpcp/pmns.c - negative_returns
Original Coverity case #98.
http://people.redhat.com/mgoodwin/pcp-cov/1/98pmns.c.html#error
Add check for ftell() returning a negative value ... sighting elephants
with purple spots has a higher probability, but this should make
coverity happy.
commit 0589401900c8733e5de48ba6945b91e23979934c
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Tue Jan 24 10:34:34 2012 +1100
libpcp/logutil.c - negative_returns
Original Coverity case #91.
http://people.redhat.com/mgoodwin/pcp-cov/1/91logutil.c.html#error
Add check for ftell() returning a negative value ... sighting flying pigs
has a higher probability, but this should make coverity happy.
commit 74ccf688dbac1fdfd928a814fbd7fe9405cc4bad
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Tue Jan 24 10:25:44 2012 +1100
libpcp/derive_fetch.c - forward_null
Original Coverity case #73.
http://people.redhat.com/mgoodwin/pcp-cov/1/73derive_fetch.c.html#error
No functional code change here, but added an assert() to ensure that
np->left is not NULL before it is dereferenced.
commit 4aa8b7d89782c620118278d5335d4f07fca60263
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Tue Jan 24 09:45:51 2012 +1100
libpcp/logutil.c - negative_returns
Original Coverity cases #90 and #100.
http://people.redhat.com/mgoodwin/pcp-cov/1/90gutil.c.html#error
and
http://people.redhat.com/mgoodwin/pcp-cov/1/100gutil.c.html#error
Check for fd (for temp file create) being negative.
commit a4aaef968b1d382b2d8d350b6ce274487bba381c
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Mon Jan 23 20:53:39 2012 +1100
libpcp/p_pmns.c - forward_null
Original Coverity case #69.
http://people.redhat.com/mgoodwin/pcp-cov/1/69p_pmns.c.html#error
Better guards around the use of the optional statuslist argument.
commit 5b84262482ac8c8ffc85146f8b937d70dc9b23c0
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Mon Jan 23 07:03:02 2012 +1100
libpcp/p_pmns.c - forward_null
Original Coverity case #68.
http://people.redhat.com/mgoodwin/pcp-cov/1/68p_pmns.c.html#error
Add explicit check for case that should never happen.
commit 04df4cd47beacd9a25d2a9c460433f4014c4aa8f
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Mon Jan 23 06:52:08 2012 +1100
libpcp/derive.c - forward_null
Original Coverity case #67.
http://people.redhat.com/mgoodwin/pcp-cov/1/67derive.c.html#error
No functional code change here, but removed a redundant test and added
an assert() to ensure the pointer in question is indeed not NULL.
Also moved the #include <assert.h> from derive.h to the two derive*.c
files to make it more obvious.
commit ee414dd26779d50e40dd58e476128cbb347dc9af
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Sun Jan 22 21:07:20 2012 +1100
libpcp/optfetch.c - forward_null
Original Coverity case #60.
http://people.redhat.com/mgoodwin/pcp-cov/1/60optfetch.c.html#error
No functinal code change here, but added an assert() to ensure the
pointers in question are indeed not NULL.
commit fd4347f21784f30852a17f541df5ffc4b40b1b12
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Sun Jan 22 07:05:13 2012 +1100
libpcp/p_result.c - deadcode removal
Original Coverity case #36.
http://people.redhat.com/mgoodwin/pcp-cov/1/36p_result.c.html#error
commit f3e61c2939b5b0c8c800c3b6dc1d7f4edab9324f
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Sun Jan 22 06:51:36 2012 +1100
libpcp/pmns.c - deadcode removal
Original Coverity case #33.
http://people.redhat.com/mgoodwin/pcp-cov/1/33pmns.c.html#error
commit 2d569de9e6246c396f2684995280ca511e519f25
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Sat Jan 21 21:42:34 2012 +1100
libpcp/pmns.c - rework deadcode removal
Redo error handling within recursive calls.
Original Coverity case #30.
http://people.redhat.com/mgoodwin/pcp-cov/1/30pmns.c.html#error
commit 470e92bbd7069b615ee057358bb8885acc1f0374
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Sat Jan 21 21:05:29 2012 +1100
libpcp/profile.c - deadcode removal
Original Coverity case #27.
http://people.redhat.com/mgoodwin/pcp-cov/1/27profile.c.html#error
commit 05c47a167f0e0bda4016edb58a8877705b381e85
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Sat Jan 21 20:58:53 2012 +1100
libpcp/profile.c - deadcode removal
Original Coverity case #26.
http://people.redhat.com/mgoodwin/pcp-cov/1/26profile.c.html#error
|