BIND 10 #1186: libdhcp implementation - DHCPv6 part

[BIND 10 Development]

#1186: libdhcp implementation - DHCPv6 part
-------------------------------------+-------------------------------------
                   Reporter:  tomek  |                 Owner:  tomek
                       Type:         |                Status:  reviewing
  enhancement                        |             Milestone:  Sprint-
                   Priority:  major  |  DHCP-20111026
                  Component:  dhcp   |            Resolution:
                   Keywords:         |             Sensitive:  0
            Defect Severity:  N/A    |           Sub-Project:  DHCP
Feature Depending on Ticket:  878    |  Estimated Difficulty:  0
        Add Hours to Ticket:  0      |           Total Hours:  0
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by tomek):

 Replying to [comment:14 stephen]:
 > Replying to [comment:8 tomek]:
 >
 > >> Putting the declarations into a enum also allows...
 > > That would be useful. Having options named exactly the same way as in
 RFC would be nice. Unfortunately, that means that there will be a conflict
 in ISC DHCP.
 > Do what you need to do for now.  In the longer-term, we need to decide
 how to handle the compatibility between Kea and DHCPv4.

 > >> '''src/lib/dhcp/libdhcp.h'''
 > >> The idea of a "LibDHCP" class is not really object oriented - it
 seems to imply a general collection of loosely-related functions, which is
 more of a C paradigm. What we have in this class seems more to be an
 !OptionCollection object - an object capable of disassembling and
 assembling
 > >>
 > >> Related to the above, there is are packOptions6() and
 unpackOptions6() methods and an !OptionFactoryRegister() method that
 requires an indication of the "universe" in which it operates. All of
 these seem to be arguing for two classes, one for v6 and one for v4, each
 with pack, unpack and factoryRegister methods.
 > >
 > > That is a work in progress. The idea is to
 > I think the rest of the comment got lost :-).  But we need to look at
 the design to see if "LibDHCP" is really a natural object.
 There's ticket 1303 for that.

 > >> '''src/lib/dhcp/libdhcp.cc'''[[BR]]
 > >> unpackOptions6(): Using the !InputBuffer class in "util" allows items
 of data to be read from a buffer in 16-bit words.
 > > !InputBuffer is unsuitable here. Hooks may modify incoming packet, so
 input buffer is actually read-write.
 > Fair point, but I think encapsulating the input data in some object that
 simplifies the packing/unpacking of data in it is worth some investigation
 at some time in the future.
 The idea here is that Pkt6 serves such purpose. We may need to look at
 unifying OutputBuffer and Pkt6 eventually.

 > >> unpackOptions6(): Should be spaces around relational operators (in
 "while (offset<end)").
 > Being pedantic, it's not usual in BIND 10 code to put spaces after an
 opening parenthesis or before a closing one.  (We tend to obey the Google
 guidelines here: http://google-
 styleguide.googlecode.com/svn/trunk/cppguide.xml#Horizontal_Whitespace)
 Fixed, but this link should be added to CodingGuidelines.

 There will be single commit with this and following comments.

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