BIND 10 #2544: DHCPv4 Options Config Parser

[BIND 10 Development]

#2544: DHCPv4 Options Config Parser
-------------------------------------+-------------------------------------
            Reporter:  marcin        |                        Owner:
                Type:  task          |  marcin
            Priority:  high          |                       Status:
           Component:  dhcpconf      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130103
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  0             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => marcin


Comment:

 Reviewed commit 393c70b7a308f5d816a87cf21e017351dbf09260.

 '''src/bin/dhcp4/config_parser.cc'''[[BR]]
 Line 469: s/than/then/

 Line 474: s/util/until/

 Line 621: I would suggest abstracting the name validation code into a
 separate method, perhaps in a base class common to both the V4 and V6
 parsers.

 Line 641: could we consider adding a method to !LibDHCP (e.g.
 getMatchingOptionDefs())to take a code value and return the range of
 relevant options?  It would eliminate the need to introduce the concept of
 an index at this level of the code. (It could also throw the configuration
 error exception if there were multiple definitions.)

 Line 830 onwards: using "else if" has the same effect and looks better.
 Alternatively, if the various buildParser calls could be logically ORed
 together: they will be called left to right until the first evaluates as
 true.  Doing that means only one "continue" is needed.

 Line 850: Should the code throw an exception if it gets here?  All parsers
 have been tried but nothing is applicable.


 '''src/bin/dhcp4/dhcp4_messages.mes'''[[BR]]
 Line 48: s/on attempt/on an attempt/

 Line 50: s/yet/but/


 '''src/bin/dhcp4/tests/config_parser_unittest.cc'''[[BR]]
 Starts with a blank line.

 Line 210: Would prefer: EXPECT_NE(0, memcmp(...)).

 Line 244: if status is expected to be zero, check "if (status != 0)"

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