BIND 10 #2637: DHCP code tidy-up for the release in January 2013

[BIND 10 Development]

#2637: DHCP code tidy-up for the release in January 2013
-------------------------------------+-------------------------------------
            Reporter:  marcin        |                        Owner:
                Type:  task          |  marcin
            Priority:  medium        |                       Status:
           Component:  dhcp          |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130122
         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 tomek):

 * owner:  tomek => marcin


Comment:

 '''src/bin/dhcp4/config_parser.cc'''
 createOption() verifies if the option_name contains spaces. Why this
 specific case is checked here, but all other checks are done in
 OptionSpace::validateName?

 '''src/bin/dhcp4/ctrl_dhcp4_srv.cc''
 establishSession(): Dumy => Dummy.

 Please update (or remove) the comment starting with:
 // We initially create ModuleCCSession() without configHandler

 The DHCP4_CONFIG_UPDATE log entry will print out full_config which
 ironically does not include full_config. merged_config should be used
 instead.

 On a related note, I think we should have a discussion with DNS team about
 how this configuration works. I'm uncomfortable with the getFullConfig not
 returning a full config. Perhaps it should be renamed to
 getComittedConfig? Or better yet it should be possible to easy obtain both
 the diff (essentially how it currently works) and the full merged config.
 Anyway, it's not something that should be done in this ticket.

 '''src/bin/dhcp6/config_parser.cc'''
 createOption() the same comment about checking against space as in its v4
 counterpart.

 '''src/lib/dhcp/option.cc'''
 Please extend the comments a bit. Something like this will do the trick:

 {{{
 -    // Write a header.
 +    // Write a header (protocol dependent - 2 bytes for DHCPv4 and 4 for
 DHCPv6)
      packHeader(buf);
 -    // Write data.
 +    // Write data (the same for both protocols)
 }}}

 '''src/lib/dhcp/option.h'''
 Comment for pack() is no longer true. This method does not return a
 pointer anymore.

 '''src/lib/dhcp/option_definition.cc'''
 OptionDefinition::writeToBuffer()
 There are no tests for adding FQDN record. If it is time consuming to add
 one, please add @todo in the test code instead.

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