BIND 10 #2837: MySQL STRICT mode causes libdhcpsrv unit tests t fail when MySQL STRICT mode is enabled

[BIND 10 Development]

#2837: MySQL STRICT mode causes libdhcpsrv unit tests t fail when MySQL STRICT
mode is enabled
-------------------------------------+-------------------------------------
            Reporter:  tmark         |                        Owner:  tmark
                Type:  defect        |                       Status:
            Priority:  medium        |  reviewing
           Component:  libdhcp       |                    Milestone:
            Keywords:                |  Sprint-DHCP-20130411
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DNS           |                 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 stephen):

 * owner:  stephen => tmark


Comment:

 '''src/lib/dhcp/hwaddr.cc'''[[BR]]
 HWAddr constructors: even single-line "if" blocks should use "{" and "}"
 (see the [wiki:CodingGuidelines#CurlyBraces coding guidelines]).

 I think you can leave the initializations of the member hwaddr_ as they
 were (i.e. created with the passed arguments) and just have the length
 check in the constructor.  At worst there is additional overhead of
 initializing hwaddr_ with the data if the exception is thrown (as opposed
 to the overhead of constructing hwaddr_ and copying data to it in separate
 steps in all cases when an exception is not thrown).

 '''src/lib/dhcp/tests/hwaddr_unittest.cc'''[[BR]]
 HWAddrTest: Please include spaces around the "+" in the initialization of
 big_data.

 HWAddrTest: It is probably better to initialize big_data explicitly (via
 std::fill or memset); I recall a problem on one system (can't remember
 which, probably Solaris) when initializing an array with fewer elements
 than the size of the array.  In fact, an even better solution is to create
 big_data_vector using:
 {{{
 vector<uint8_t> big_data_vector(HWAddr::MAX_HWADDR_LEN + 1, 0);
 }}}
 (although the "0" is not really necessary - it's the default) and, when
 testing the constructor that requires a base address and size, pass
 {{{&big_data_vector[0]}}} and {{{big_data_vector.size()}}} through as
 arguments.

 '''src/lib/dhcpsrv/mysql_lease_mgr.cc'''[[BR]]
 openDatabase: Suggest that the "set" in the command setting the session
 variable be uppercase (matches the style of the prepared statements).


 '''src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc'''[[BR]]
 Line 884/906: not sure about the reformatting here.  The BIND 10 standard
 for continuation lines is the [wiki:CodingGuidelines#TabsIndentation BSD
 style].

 '''!ChangeLog entry'''[[BR]]
 Would rewrite the last sentence as:
 {{{
 Also, attempts to create a HWAddr (hardware address) object with
 too long an array of data now throw an exception.
 }}}

 Only needs one git commit number, that of the commit in which the change
 has been merged into master.

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