BIND 10 #613: Address CppCheck issues

BIND 10 Development do-not-reply at isc.org
Mon Feb 28 22:46:44 UTC 2011


#613: Address CppCheck issues
-------------------------------------+-------------------------------------
                 Reporter:  stephen  |                Owner:  jinmei
                     Type:  task     |               Status:  reviewing
                 Priority:  major    |            Milestone:  A-Team-
                Component:           |  Sprint-20110309
  Unclassified                       |           Resolution:
                 Keywords:           |            Sensitive:  0
Estimated Number of Hours:  3.0      |  Add Hours to Ticket:  0
                Billable?:  1        |          Total Hours:  0
                Internal?:  0        |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:8 jinmei]:
 > > The dns_service ones are most probably false alarms.
 >
 > As we discussed on jabber, it seems to be due to differences between
 > cppcheck 1.46 and 1.47.  It would be better to ensure clean result with
 > various versions, but it would also be better to keep the suppress list
 > shorter.  Since in this case the noisy lines are small and it won't be
 > triggered on our target buildbot box, I'd leave the false alarms on
 > dns_service open.  For redundant assignments, see below.

 OK

 > > Also, what is the benefit of using a vector instead of constant-size
 array as a buffer which size doesn't change?
 >
 > Nothing except it can be initialized in the member initialization list
 > (or in this case implicit initialization should also work).  Since the
 > warning on initialization in this case is mostly a false positive
 > (there doesn't seem to be a place where the buffer is used before
 > set), another option is to exclude this case in the suppress list.
 > But I thought changing the code was still better than suppressing the
 > warning because the changed code looked as readable as the original
 > one to me.  Also, since this is only for tests, overheads with
 > std::vector shouldn't be a problem.

 I don't really mind it too much, it's just I would never write such code,
 so I was curious if there's some advantage. I'd trust the compiler in
 this, it should be simple enough for it to completely optimise it out.

 > Later in my work on this branch, however, I also realized I could
 > address the initialization warning by explicitly setting the variable
 > in the body of the constructor (just like I did in commit 0919193).
 > So, if you don't like vector, I'm okay with addressing this part that
 > way.

 No need.

 It seems OK, merge it, please.

 Thanks

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


More information about the bind10-tickets mailing list