BIND 10 #2374: MasterLexer's Number state classes

BIND 10 Development do-not-reply at isc.org
Mon Nov 26 11:12:58 UTC 2012


#2374: MasterLexer's Number state classes
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  muks
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121204
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  libdns++                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  4
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  loadzone-ng                        |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => muks


Comment:

 thanks for the review :)

 > * What happens if the lexer is at `"1234;my_comment"` (without quotes)?
 Is it parsed as a number?
 >

 it is now, good catch :)

 > * Please add tests for the escaped cases.
 >

 done (also split them up into two sets of tests as it was getting quite
 long)

 > * `strtoul()` accepts a signed `int` type for base, so its type should
 be updated.
 >

 fixed

 > * Can we add this comment to `master_lexer.cc`:
 > {{{
 > +++ b/src/lib/dns/master_lexer.cc
 > @@ -252,7 +252,7 @@ const CRLF CRLF_STATE;
 >  const String STRING_STATE;
 >  const QString QSTRING_STATE;
 >  const Number NUMBER_STATE;
 > -}
 > +} // unnamed namespace
 > }}}
 >

 added (to the other unnamed namespace snippets as well)

 > * This is the first time I'm looking at the string and number lexers.
 Though it is offtopic to this bug, I think the string tests in some places
 miss comparing the token's values such as in `MasterLexerStateTest.crlf`
 (from basic checking, the code it's testing is correct though).
 >

 it is tested a number of times, but perhaps not in every occurrence. I
 added one in the first change for comments-following-number btw

 > * Is there a ticket to handle the `\DDD` syntax (RFC1035 sec 5.1)?

 I don't think so; IIRC we are skipping that for now (btw, while it is
 represented as a number, what is being parsed then is a string character,
 so they shouldn't go into the number handler anyway)

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


More information about the bind10-tickets mailing list