BIND 10 #2275: work on valgrind backlog

BIND 10 Development do-not-reply at isc.org
Thu Oct 4 21:23:37 UTC 2012


#2275: work on valgrind backlog
-------------------------------------+-------------------------------------
                   Reporter:  jreed  |                 Owner:  jinmei
                       Type:         |                Status:  reviewing
  defect                             |             Milestone:
                   Priority:         |  Sprint-20121009
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  Unclassified                       |           Sub-Project:  Core
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 '''acl/dns.cc'''

 Does the change is to ensure that the loader will be destroyed
 at the end of program's lifetime?  If so, is this safe?  We cannot
 control the order of destruction of these objects, so I'm afraid this
 can cause unexpected disruption due to possible reference between
 these objects.  For example, what if there's another static object,
 which internally refers to this static loader, and its destructor
 assumes the loader is valid?

 ...hmm, but on further thought it's not specific to this case...this
 can be a potential issue wherever we use this type of static proxy
 objects.  I'd be still concerned about it with such a complicated
 class than a mostly straightforward value-class like `RRType`, but
 that'd be a separate issue anyway.

 So I'm okay with the change with a couple of suggestions:

 - I'm afraid the intent of the use of smart pointer is not so obvious,
   so I'd comment why we do it.
 - I'd use boost::scope_ptr instead of auto_ptr.  In this case the
   former should be sufficient.

 '''sqlite3_accessor_unittest.cc'''

 Not really related to the branch, but this comment doesn't seem to be
 correct:
 {{{#!cpp
     // should work now that we closed it
     SQLite3Accessor accessor3(SQLITE_NEW_DBFILE, "IN");
 }}}
 Looks like a copy-paste error.

 '''labelsequence_unittest.cc'''

 My valgrind (3.8.1) doesn't complain about this.  What's wrong with
 the original code?

 '''recursive_query_xxx'''

 Not directly related, I'd make the 'const char*' variables 'const
 char* const'.  I'd also define things in an unnamed namescope.

 '''socket_requestor_test.cc'''

 My valgrind (3.8.1) doesn't complain about this.  What's wrong with
 the original code?

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


More information about the bind10-tickets mailing list