BIND 10 #3145: PD: Add support for IA_PD and IAPREFIX options

BIND 10 Development do-not-reply at isc.org
Fri Sep 6 13:48:59 UTC 2013


#3145: PD: Add support for IA_PD and IAPREFIX options
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:  tomek
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcp6         |                    Milestone:
            Keywords:                |  Sprint-DHCP-20130918
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  0             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tmark):

 * owner:  tmark => tomek


Comment:

 General question:

 We have a class IOAddress (for better or worse), are we or should we have
 a class for IPv6Prefix?  This is perhaps a larger picture question than
 this ticket's scope.

 -------------------------------------------
 option6_iaprefix.h

 "The major differences are fields order and prefix has also additional
 prefix length field."

 This doesn't quite make sense. Could you rephrase?

 -------------------------------------------
 option6_iaprefix.h

 I have been told that Doxygen comments should use the word Constructor,
 not  "Ctor" or "ctor".

 -------------------------------------------
 option6_iaprefix.h

  "It make everyone's like much easier,"

  I think you meant:

  "It makes everyone's life much easier"

 -------------------------------------------

 option6_iaprefix.h

 line spacing is "non-standard":

 {{{
     /// @return string with text representation.
     virtual std::string
     toText(int indent = 0);
 }}}

 -------------------------------------------

 option6_iaprefix.h

 pack and unpack methods can both throw but it is not mentioned in the
 header commentary.
 In the case of the latter, a throw in unpack renders the contents of the
 object invalid,
 the user should be made aware of this.

 -------------------------------------------

 option6_iaprefix_unittests.cc

 The unit testing only tests the "sunny path".  It does not test invalid
 address in the packet,
 which should case pack() to throw, or an invalid distance which should
 cause unpack() to throw.

 Also, the constructor invocation that is there, should probably in an
 ASSERT_NO_THROW() because
 if it throws the test will blow up.

 ----------------------------------------------

 option6_ia_unittests.cc

 line 79 and 211, constructors should be within an ASSERT_NO_THROW(), yes?
 line 85, pack() should also be in an ASSERT_NO_THROW?


 -----------------------------------------------

 Do we need a !ChangeLog entry for this?



 valgrind and cppcheck had no complaints.

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


More information about the bind10-tickets mailing list