BIND 10 #3222: DHCP6 should send generated NameChangeRequests to b10-dhcp-ddns

[BIND 10 Development]

#3222: DHCP6 should send generated NameChangeRequests to b10-dhcp-ddns
-------------------------------------+-------------------------------------
            Reporter:  tmark         |                        Owner:  tmark
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcp-ddns     |                    Milestone:  DHCP-
            Keywords:                |  Kea0.9-alpha
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  32            |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by marcin):

 * hours:  5 => 0
 * owner:  marcin => tmark


Comment:

 Reviewed commit 50f472025f22aa3afab6a699de50ff8ccfa8022c

 I fixed a few nits in the code and fixed it. Please pull these changes
 from the branch.

 '''src/bin/d2/nc_trans.h'''
 returnString could be const.

 Also, there is no unit test for this function. The function is simple so I
 am not fully convinced that it is needed, but perhaps you just forgot to
 add it? ;)

 '''src/bin/dhcp6/dhcp6_srv.cc'''
 In line 1077 you create an empty pointer and then reset it to the actual
 pointer to NCR. Why? You could just pass the raw pointer to the
 constructor to avoid double initialization.

 I wonder if it wouldn't be more logical if the following debug message:
 {{{
         LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL,
 DHCP6_DDNS_CREATE_ADD_NAME_CHANGE_REQUEST).arg(ncr->toText());

 }}}

 was moved above this:
 {{{
         // Post the NCR to the D2ClientMgr.
         CfgMgr::instance().getD2ClientMgr().sendRequest(ncr);
 }}}

 If the sendRequest experiences an error, it will log the error message and
 return. Next, the debug message will be displayed informing that we have
 been sending the NCR. If you swap the two as I propose, first you'll get
 the debug message saying that we are sending NCR and if it fails, the
 error message, which makes more sense for me.

 The same applies to createRemovalNameChangeRequest

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