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