BIND 10 #2209: define and implement ConfigurableClientList::getCacheZoneUpdater()
BIND 10 Development
do-not-reply at isc.org
Fri Oct 26 21:04:43 UTC 2012
#2209: define and implement ConfigurableClientList::getCacheZoneUpdater()
-------------------------------------+-------------------------------------
Reporter: | Owner: jinmei
jinmei | Status: reviewing
Type: task | Milestone:
Priority: | Sprint-20121106
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: |
background zone loading |
Add Hours to Ticket: 0 |
Internal?: 0 |
-------------------------------------+-------------------------------------
Comment (by jinmei):
Replying to [comment:13 vorner]:
> > I'm not sure about the latter, but as I said I can live with it as an
> > intermediate workaround. For the former, I think we need to run one
> > more review cycle after the cleanup. BTW, #2208 now seems to be ready
> > for merge.
>
> I tried merging it. The changes are pushed now. The merge was little bit
> interesting. The merge commit contains nothing substantial, just the
conflict
> resolution and some slight cleanups (constification) I noticed on the
way. The
> adjustments to tests and the code are in follow-up commits.
>
> I got rid of the getZoneTableSegment too.
The merge and after merge changes look basically okay. I also
confirmed the previous concerns were resolved.
Some minor comments:
- I suggest renaming `DataSourceInfo::segment_` something like
`ztable_segment_` because "segment" can be used in several different
contexts. Same for the constructor parameter, etc, too.
- I guess we can now remove `ConfigurableClientList::mem_sgmt_`.
- I'd leave the same (sense of) note about the use of assert()
in the `ZoneTableSegmentLocal` destructor as this one:
{{{#!cpp
// leak. assert() (with abort on failure) may be too harsh, but
// it's probably better to find more leaks initially. Once it's
stabilized
// we should probably revisit it.
}}}
which is currently in the `ConfigurableClientList` destructor (and
will be removed if we adopt the second suggestion)
With these I think the branch can be merged.
--
Ticket URL: <http://bind10.isc.org/ticket/2209#comment:14>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list