BIND 10 #2726: New cppcheck reports

[BIND 10 Development]

#2726: New cppcheck reports
-------------------------------------+-------------------------------------
            Reporter:  muks          |                        Owner:
                Type:  defect        |  vorner
            Priority:  medium        |                       Status:
           Component:  Unclassified  |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130625
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by muks):

 * owner:  muks => vorner


Comment:

 Hi Michal

 Replying to [comment:9 vorner]:
 > I tried to add `-I./ext/asio` too, but then it run for more than half of
 hour, which seems too much.

 Nod. Even I observed this by adding the system include paths. We should
 not let it run for so long.

 > Also, I upgraded to cppcheck 1.60.1 in between. I fixed some of what it
 reported, I think, but there are 3 groups of reports that I left unfixed,
 and I think we should create a new ticket for them:
 >  * Thinks like this:
 >  * `src/lib/util/random/random_number_generator.h:113: check_fail:
 >  * Assert statement calls a function which may have desired side
 >  * effects:
 >  * 'areProbabilitiesValid'. (warning,assertWithSideEffect)`. They are
 >  * invalid, as these functions have no side effects.

 The point of this cppcheck report is that `assert()`s should not be let
 to call any methods (or have any other side effects such as updating
 variables, etc.) within the assert macro at all. This is because
 `NDEBUG` influences whether the code is executed or not.

 If we want to check the result of a call, the code should be as such:

 {{{
    const bool status = someFunctionCall();
    assert(status);
 }}}

 The fact that in our case these methods don't have side effects is a
 bonus, but the code should be updated nevertheless as it's bad coding
 style.

 >  * `src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc:748: check_fail:
 >  * Array 'leases[2]' accessed at index 2, which is out of
 >  * bounds. (error,arrayIndexOutOfBounds)` ‒ this is dhcp code.

 We should ask DHCP devs to look at this. If you are going to create a
 ticket for the remaining cppcheck issues, also create a separate DHCP
 specific ticket with this so we can assign it to them.

 >  * Unmatched suppressions ‒ it seems newer cppcheck removed some
 >  * checks. I'm not sure what to do about these not to introduce
 >  * warnings to the previous versions ‒ maybe suppress the warning
 >  * about unmatched suppression.

 :)

 I have pushed a minor comment update commit. Please check it. The work
 in the branch looks good to me.

-- 
Ticket URL: <http://bind10.isc.org/ticket/2726#comment:11>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development