Opened 9 years ago

Closed 8 years ago

#1193 closed defect (fixed)

New cppcheck adds new recommendations

Reported by: vorner Owned by: jelte
Priority: medium Milestone: Sprint-20120306
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: Core Feature Depending on Ticket:
Estimated Difficulty: 5 Add Hours to Ticket: 0.6
Total Hours: 0 Internal?: no

Description

When running cppcheck locally, I discovered it lists bunch of errors. It turns out that I have version 1.49 and the buildbot has 1.47. Jeremy said he would upgrade it. But the issues should be solved first.

Here's what I get. Some of them might be tests (for example Same expression on both sides) and should be ignored, but some look like real issues (the uninitialized values):

src/lib/datasrc/tests/database_unittest.cc:673: check_fail: Possible inefficient checking for 'expected_rdatas' emptiness. (performance,stlSize)
src/lib/datasrc/tests/database_unittest.cc:678: check_fail: Possible inefficient checking for 'expected_sig_rdatas' emptiness. (performance,stlSize)
src/lib/dns/tests/name_unittest.cc:546: check_fail: Same expression on both sides of '<='. (style,duplicateExpression)
src/lib/dns/tests/name_unittest.cc:558: check_fail: Same expression on both sides of '<='. (style,duplicateExpression)
src/lib/dns/tests/name_unittest.cc:569: check_fail: Same expression on both sides of '<'. (style,duplicateExpression)
src/lib/dns/tests/name_unittest.cc:580: check_fail: Same expression on both sides of '<'. (style,duplicateExpression)
src/lib/dns/tests/name_unittest.cc:293: check_fail: Unmatched suppression: selfAssignment (information,unmatchedSuppression)
src/lib/dns/tests/rdata_unittest.cc:228: check_fail: Unmatched suppression: selfAssignment (information,unmatchedSuppression)
src/lib/dns/tests/rrttl_unittest.cc:141: check_fail: Same expression on both sides of '<='. (style,duplicateExpression)
src/lib/dns/tests/rrttl_unittest.cc:153: check_fail: Same expression on both sides of '<='. (style,duplicateExpression)
src/lib/dns/tests/rrttl_unittest.cc:164: check_fail: Same expression on both sides of '<'. (style,duplicateExpression)
src/lib/dns/tests/rrttl_unittest.cc:175: check_fail: Same expression on both sides of '<'. (style,duplicateExpression)
src/lib/dns/tests/tsigkey_unittest.cc:137: check_fail: Unmatched suppression: selfAssignment (information,unmatchedSuppression)
src/lib/nsas/glue_hints.cc:89: check_fail: Possible inefficient checking for 'addresses_v4' emptiness. (performance,stlSize)
src/lib/nsas/glue_hints.cc:90: check_fail: Possible inefficient checking for 'addresses_v6' emptiness. (performance,stlSize)
src/lib/nsas/hash_table.h:62: check_fail: Member variable 'HashTableSlot<T>::mutex_' is not initialized in the constructor. (warning,uninitVar)
src/lib/nsas/hash_table.h:62: check_fail: Member variable 'HashTableSlot<T>::list_' is not initialized in the constructor. (warning,uninitVar)
src/lib/nsas/hash_table.h:62: check_fail: Member variable 'HashTableSlot<T>::mutex_' is not initialized in the constructor. (warning,uninitVar)
src/lib/nsas/hash_table.h:62: check_fail: Member variable 'HashTableSlot<T>::list_' is not initialized in the constructor. (warning,uninitVar)
src/lib/nsas/hash_table.h:62: check_fail: Member variable 'HashTableSlot<TestEntry>::mutex_' is not initialized in the constructor. (warning,uninitVar)
src/lib/nsas/hash_table.h:62: check_fail: Member variable 'HashTableSlot<TestEntry>::list_' is not initialized in the constructor. (warning,uninitVar)
src/lib/nsas/hash_table.h:62: check_fail: Member variable 'HashTableSlot<TestEntry>::mutex_' is not initialized in the constructor. (warning,uninitVar)
src/lib/nsas/hash_table.h:62: check_fail: Member variable 'HashTableSlot<TestEntry>::list_' is not initialized in the constructor. (warning,uninitVar)
src/lib/nsas/tests/nameserver_entry_unittest.cc:142: check_fail: Possible inefficient checking for 'vec' emptiness. (performance,stlSize)
src/lib/nsas/tests/nameserver_entry_unittest.cc:177: check_fail: Possible inefficient checking for 'vec' emptiness. (performance,stlSize)
src/lib/nsas/hash_table.h:62: check_fail: Member variable 'HashTableSlot<T>::mutex_' is not initialized in the constructor. (warning,uninitVar)
src/lib/nsas/hash_table.h:62: check_fail: Member variable 'HashTableSlot<T>::list_' is not initialized in the constructor. (warning,uninitVar)
src/lib/nsas/zone_entry.cc:332: check_fail: Possible inefficient checking for 'probabilities' emptiness. (performance,stlSize)
*:0: check_fail: Unmatched suppression: debug (information,unmatchedSuppression)
*:0: check_fail: Unmatched suppression: missingInclude (information,unmatchedSuppression)
:: check_fail: Cppcheck cannot find all the include files (use --check-config for details) (information,missingInclude)

Subtickets

Attachments (1)

cppcheck (7.1 KB) - added by vorner 8 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 9 years ago by shane

  • Milestone changed from New Tasks to Next-Sprint-Proposed

Changed 8 years ago by vorner

comment:2 Changed 8 years ago by vorner

  • Milestone set to Next-Sprint-Proposed

I updated to cppcheck 1.51 and more suggestions appeared, some of them look valid (like the check for emptyness). Should we do something with it? I just can't run cppcheck locally, because of too much noise. I'm putting it to next-sprint-proposed, because I'd like to have few more local checks.

The new output is attached.

comment:3 Changed 8 years ago by jelte

  • Estimated Difficulty changed from 0 to 5

comment:4 Changed 8 years ago by jelte

This suddenly annoyed me enough to do something about it.

The missingIncludes error at the end there turned out to be a bug in cppcheck itself (fixed somewhere around 1.51 or 1.52, certainly present in 1.49), so I upgraded to 1.53 and fixed any additional issues encountered.

Also added a few more suppressions, in some tests we intentionally do stupid things that are caught now (like a.compare(a) == 0)

Anyway, slightly ahead of the sprint, but with the changes in trac1193 cppcheck (1.53) succeeds again.

I'll wait a day before I put it up for review :)

Note: some of the suggestions in the attachment did not appear for me

comment:5 Changed 8 years ago by jelte

  • Milestone changed from Year 3 Task Backlog to Sprint-20120306

comment:6 Changed 8 years ago by jreed

I upgraded the automated checks to cppcheck 1.53.

I misunderstood in jabber about the version and thought it would fix the single existing problem and didn't know it would introduce all these (I forgot and thought this was already done).

So please merge soon! :)

comment:7 Changed 8 years ago by shane

  • Owner set to shane
  • Status changed from new to reviewing

comment:8 Changed 8 years ago by shane

  • Add Hours to Ticket changed from 0 to 0.6
  • Owner changed from shane to jelte
  • Sub-Project changed from DNS to Core

Looks good, please merge.

comment:9 Changed 8 years ago by jelte

  • Resolution set to fixed
  • Status changed from reviewing to closed

Thanks, merged, closing ticket.

Note: See TracTickets for help on using tickets.