BIND 10 #2377: define dns::MasterLoad class
BIND 10 Development
do-not-reply at isc.org
Wed Dec 12 17:59:05 UTC 2012
#2377: define dns::MasterLoad class
-------------------------------------+-------------------------------------
Reporter: jinmei | Owner:
Type: task | jinmei
Priority: medium | Status:
Component: libdns++ | reviewing
Keywords: | Milestone:
Sensitive: 0 | Sprint-20121218
Sub-Project: DNS | Resolution:
Estimated Difficulty: 7 | CVSS Scoring:
Total Hours: 0 | Defect Severity: N/A
| Feature Depending on Ticket:
| loadzone-ng
| Add Hours to Ticket: 0
| Internal?: 0
-------------------------------------+-------------------------------------
Comment (by jinmei):
Replying to [comment:28 vorner]:
> > Besides, the readability argument doesn't seem to be fair, at least
> > not yet. As I said before, we also need to handle the `INITIAL_WS`
> > case. So we cannot simply do:
> > {{{#!cpp
> > const MasterToken::StringRegion&
> > name_string(lexer_.getNextToken(MasterToken::QSTRING).
> > getStringRegion());
> > }}}
> > but need some kind of if-else or switch-case anyway. I'm okay with
> > deferring the actual handling of it to a separate ticket, but if you
> > want to claim better readability, we need to discuss it with taking
> > into account the resulting form of code that handles deferred cases.
>
> Hmm, I did want to defer it to #2427. But if you ask, I'd envision
something
> like this. The loop does not create a new token in each iteration, but
uses one
> mutable variable. That variable is then left intact when leaving the
loop.
>
> After the loop, we handle the string, the initial whitespace and leave
> everything else as an error. This will get rid of ungetting the token,
but
> it'll also separate the skipping of newlines part from the name handling
part.
Okay, at this point it's probably not good to hold this ticket too
long. Please merge this branch at this point, and let's revisit the
discussion in #2427.
As for this:
> > > Anyway, the current code does handle the EOF even in successful
case, as it
> > > calls the function to create Rdata ‒ it does that. I added a test to
confirm it.
> >
> > I don't think we need a test for that case in `MasterLoader`; it's
> > beyond the responsibility to it (As commented above, I misunderstood
> > the original BIND 9 code on this point). I suggest removing the
> > noEOLN test.
>
> Is there any reason to remove the test, when it's already written?
I prefer removing it (see my other ticket message), but I'd leave it
to you, too.
--
Ticket URL: <http://bind10.isc.org/ticket/2377#comment:30>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list