BIND 10 #1305: auth NSEC support: some more updates to data source

[BIND 10 Development]

#1305: auth NSEC support: some more updates to data source
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20111025
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => jinmei


Comment:

 > Do you mean passing as !DatabaseAccessor?& or as
 boost::shared_ptr<!DatabaseAccessor?>&?
 The latter.

 > In the latter case we'll have to dereference it to acquire the ownership
 anyway for the same reason, so I suspect the actual benefit of saving the
 copy is marginal
 Well, there is work to do in incrementing and decrementing a reference
 count.  If passed by value, a copy is made within the function (causing an
 increment of the reference count) then that copy is passed as argument to
 the accessor_ copy constructor.  Finally, the copy is discarded (a
 decrement of the reference count).  You are right in the the cost saving
 is marginal, but we use shared pointers extensively in BIND 10 and all the
 costs mount up.

 > (in this specific usage, in particular, I guess the compiler will
 combine the multiple copy constructions involved in the function call and
 the member variable initialization, and the actual benefit would actually
 be zero).
 Bear in mind that a copy constructor could contain code unrelated to the
 copy (which may be a bad idea but is not prohibited by the C++ standard),
 so if the compiler were to do that it could be break required behaviour.
 However, since the boost code is mainly headers, it could be that the
 compiler is clever enough to see that there are no external calls and to
 optimise the copy away.  But we don't know that.

 > In general, I'd be careful about passing a shared pointer in the form of
 reference because it could often lead to break the "shared" nature of the
 object. If the called function handles the passed reference carelessly
 (e.g., keep storing it somewhere in the form of reference) it can easily
 lead to dangling ownership. But such a discussion would be far beyond the
 scope of this ticket.
 I'm not sure I follow your line of reasoning here.  A function keeping a
 pointer (or reference) to a passed-in object if there is no guarantee that
 the object will remain in existence when the pointer/reference is next
 used is a problem regardless of the object being passed in.  In this case,
 there is no guarantee about the lifetime of the shared_ptr object (other
 than it will exist for the duration of the call to this method), so the
 function shouldn't store a pointer or reference to it.

 I'll leave it up to you whether you change the method signature.  The code
 is OK to merge.

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