BIND 10 #2355: DHCP v4 and v6 parsers' unification should be considered

[BIND 10 Development]

#2355: DHCP v4 and v6 parsers' unification should be considered
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:
                Type:  defect        |  UnAssigned
            Priority:  medium        |                       Status:
           Component:  dhcpconf      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130411
         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 tmark):

 * owner:  tmark => UnAssigned
 * status:  assigned => reviewing


Comment:

 This initial commit is an interim step in the refactoring effort to unify
 DHCP4 and DHCP6 parser code.  It moves the bulk of the parser classes
 formerly
 declared in src/bin/dhcp4/config_parser.cc and
 src/bin/dhcp4/config_parser.cc
 into two new files in lib/dhcpsrv:

     dhcp_parsers.h and dhcp_parsers.cc

 A few classes remain, that provide further opportunity for refactoring
 In order to reduce the amount of change to review further refactoring
 and unit tests will be handled as additional commits.

 The primary challenge in developing common parser classes is the use
 of references to global storage structures (e.g. uint32_values,
 string_values,
 option_def_intermediate) inside the parser implementations.  These globals
 provide the outermost storage context and are specific to the server
 instance itself.  Removing the need for these references was essential.

 For the most part, these references establish a default storage
 for parser instances which may then be altered with a subsequent call to
 the parser's "setStorage" method.  Analysis of the current implementation
 reveals that the storage context required for a particular parser is
 known at the time the parser is instantiated and that parsers do not
 migrate from one storage context to another.  Therefore passing the
 storage value as a parameter into the constructor seems appropriate.

 The current implementation, separates the parser construction
 and setting the storage context into separate stages. It first uses a map
 of parser factory methods to instantiate the parser, and then in a
 subsequent
 step uses dynamic-cast driven decision-making to set the parser's storage
 (See
 Subnet4ConfigParser build, createSubnet4ConfigParser, and buildParser
 methods).

 This technique was replaced with a more direct approach.
 createSubnet4ConfigParser
 now constructs the parsers directly (not via factories), passing the
 appropriate
 storage context into the constructor.  This eliminates the need for
 buildParse
 function template, and reduces the code in the build() method.

 The use of factory-map mechanism used in the function
 createGlobalDhcp4ConfigParser()
 as been similarly modified.  This was code was also replaced to allow
 use of the new parser constructors to set the parser storage.

 The refactored parsers all define a private default constructors, and
 enforce
 that the storage parameter passed into their public constructor not be
 NULL.

 These same changes were applied the dhcp6.

 This commit is # 53ceb71e36d310a2a4cd5fef7ae9fa5d410f078a

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