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