On 01/28/2012 09:01 PM, Ken McDonell wrote:
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.
#30 looks ok, Commit 2d569de
#32 agree: No issue, Coverity analysis is wrong (and weird)
#325 agree: Coverity is wrong: a successful pmNameID() will always
allocate p so it can't be already freed.
#327 agree: Coverity is probably wrong, but it's a hairy one.
If paranoidLogRead fails, then nrp will be unaltered and
possibly still pointing at storage that's freed by
pmFreeResult(rp) on the next iteration. But if paranoidLogRead
fails, we break out of the loop, so I don't see how there
is a problem (and my head hurts :P)
#337 agree: Coverity is being pedantic. *rp = lfup->rp; is fine
even if lfup->rp == NULL
#345 I guess Coverity is complaining that if (f != lcp->l_mfp)
then we fclose(f) without checking if f is NULL first.
There are several places where we fclose(f) but don't
set it to NULL afterwards, e.g. near line 1973 in current
top-of-tree dev .. perhaps we shoudl be?
Also, the (unrelated) patch commit ceb12dd seems OK.
#349 and #350 agree: we're just assigning fp = fp->next and freeing
the original fp. Coverity has it's nickers in a twist
Cheers
-- Mark
|