[pcp] pcp updates - Coverity changes for libpcp

Mark Goodwin mgoodwin at redhat.com
Wed Feb 1 18:53:55 CST 2012


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




More information about the pcp mailing list