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

BIND 10 Development do-not-reply at isc.org
Tue Apr 10 07:33:22 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      |
-------------------------------------+-------------------------------------
Changes (by kevin_tes):

 * owner:  kevin_tes => jinmei


Comment:

 Replying to [comment:8 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?
 Yeah, it  okay now please merge.:)

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


More information about the bind10-tickets mailing list