BIND 10 #1791: update InMemoryZoneFinder::load() to support sqlite3 backend

BIND 10 Development do-not-reply at isc.org
Mon Apr 9 20:37:40 UTC 2012


#1791: update InMemoryZoneFinder::load() to support sqlite3 backend
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120417
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:  xfr    |
  for in-memory                      |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:7 kevin_tes]:

 > I have check the changed codes twice, and it seems good to me.

 Thanks for the review.

 >
 > But I am not sure a detail below:
 > diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
 > {{{#!cpp
 > +        const ConstRdataPtr rdata_base =
 > +            rdata::createRdata(rtype, class_, rdata_);
 > +        ConstRdataPtr rdata;
 > +        while (data_ready_) {
 > +            bool same_type = true;
 > +            if (rdata) { // for subsequent data, replace it with the
 new RDATA.
 > +                const RRType next_rtype(rtype_);
 > +                rdata = rdata::createRdata(next_rtype, class_, rdata_);
 > +                same_type = isSameType(rtype, rdata_base, next_rtype,
 rdata);
 > }}}
 > The 'rdata' declares without definition,but it was used in the
 'if(rdata)',i have no idea about this.
 > Otherwise,it can be merged.

 `rdata` is defined in line 3 in the above quoted block.

 But I guess the more fundamental issue is the (un)readability of this
 method.  Although I'd say it was unreadable even before my changes in
 the branch, I see it could easily confuse readers.

 I tried to cleanup the code with more comments so it would be less
 confusing.  And, while doing so, I found another bug in the
 implementation (which existed before this branch), so I fixed it, too.

 I'd propose adding another separate changelog entry for this part of
 bug fix.

 {{{
 423.?   [bug]           jinmei
         The database based zone iterator now correctly resets mixed TTLs
         of the same RRset (when that happens) to the lowest one.  The
         previous implementation could miss lower ones if it appears in a
         later part of the RRset.
         (part of Trac #1791, git f1f0bc00441057e7050241415ee0367a09c35032)
 }}}

 Could you check the latest branch to confirm the bug fix and see if
 it's generally okay now?

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


More information about the bind10-tickets mailing list