BIND 10 #2377: define dns::MasterLoad class

[BIND 10 Development]

#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
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:22 jinmei]:
 > 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.

 Replying to [comment:26 jinmei]:
 > > 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? The
 current
 behaviour makes sense and I think we do want it (even if it was different
 than
 bind9, we would be different in a warning, which wouldn't cause any
 disruption,
 but would make some people clean up their master files). And the fact you
 asked
 for that handling and it wasn't obvious it is handled this way means it is
 not
 trivial side effect of the code, so an explicit test may be actually a
 good thing.

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