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

[Michal Hlavinka]

 > 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