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