Nut upsdev - Jan 2012 - [nut-commits] svn commit r3383 - branches/coverity

Hi Michal

2012/1/10 Michal Hlavinka <mihl-guest at alioth.debian.org>
> Author: mihl-guest
> Date: Tue Jan 10 09:10:04 2012
> New Revision: 3383
> URL: http://trac.networkupstools.org/projects/nut/changeset/3383
>
> Log:
> Creating a branch for Coverity reported problems
>
I'm very interested there!
Have you been able to get NUT part of the Coverity Scan program, or are you
running Coverity on your own?

I'd love to see a summary report, to get an overall idea of the situation.

cheers,
Arnaud
-- 
Linux / Unix Expert R&D - Eaton - http://powerquality.eaton.com
Network UPS Tools (NUT) Project Leader - http://www.networkupstools.org/
Debian Developer - http://www.debian.org
Free Software Developer - http://arnaud.quette.free.fr/
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.alioth.debian.org/pipermail/nut-upsdev/attachments/20120110/267451ee/attachment.html>

> Hi Michal
Hi Arnaud

 > >     Log:
 > >     Creating a branch for Coverity reported problems
 >
 >
 > I'm very interested there!
 > Have you been able to get NUT part of the Coverity Scan program, or are
 > you running Coverity on your own?

we have bought Coverity license, so I've checked it on my own.

 >
 > I'd love to see a summary report, to get an overall idea of the 
situation.
 >
 > cheers,
 > Arnaud

I was thinking what's the best way to send you several patches. I think 
new branch is the easiest way to do so.

I've parsed Coverity scan result, removed false positives and fixed 
several issues. Not all of them are bugs, so check that branch and pick 
those you think are worthy. Coverity reports a lot of issues that are 
only "not that pretty code, but not worth to change it". So it's
always
a question where you should draw the line :)

I've fixed everything I've found worth to fix, except 2 issues I
don't
know whether/how to fix. There are also a lot of reports about sprintf 
(Coverity reports every sprintf as "possibly dangerous", but hard to 
tell if it's worth it to fix it. I've checked all of them if there was 
any obvious issue.)

The two issues I did not fix:
1) Error: REVERSE_INULL:
clients/upsmon.c:789: deref_ptr_in_call: Dereferencing pointer "un". 
(The dereference is assumed on the basis of the 'nonnull' parameter 
attribute.)
clients/upsmon.c:795: check_after_deref: Dereferencing "un" before a 
null check.

I don't know if 'un' can be null, but it is checked for null after 
strcmp, if it can really be null, it should be checked before strcmp

2) Error: CONSTANT_EXPRESSION_RESULT: (4x)
drivers/libshut.c:731: result_independent_of_operands: (Start[0] & 0x80) 
== 5 is always false regardless of the values of its operands. This 
occurs as the logical operand of if.

and exactly the same thing at
libshut.c:973,
mge-shut.c:527,
mge-shut.c:839,

in libshut.c and mge-shut.h, there are
#define SHUT_TYPE_NOTIFY        0x05
#define SHUT_PKT_LAST           0x80

so if ((c & SHUT_PKT_LAST) == SHUT_TYPE_NOTIFY) can't be true
(c & 0x80) == 0x05


And that's all for first round :) Let me know if you have any question.

Cheers
Michal