BIND 10 #2749: kill io_utilities.h or make it safe

BIND 10 Development do-not-reply at isc.org
Fri Jan 24 18:59:28 UTC 2014


#2749: kill io_utilities.h or make it safe
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:  muks
                Type:  defect        |                       Status:
            Priority:  medium        |  reviewing
           Component:  Unclassified  |                    Milestone:
            Keywords:                |  Sprint-20131015
           Sensitive:  0             |                   Resolution:
         Sub-Project:  Core          |                 CVSS Scoring:
Estimated Difficulty:  3             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => muks


Comment:

 Reviews commit c7aeb4bec30cf6373e77216058174d6d35850dde

 '''src/bin/dhcp6/dhcp6_srv.cc'''[[BR]]
 generateServerID(): the construct "srvid.size() - n" is used to ensure
 that the buffer is large enough for the writeUintNN() calls.  However, as
 the preceding line declares the !OptionBuffer with a size of at least 8, I
 think this construct - although general - is unnecessary.  I suggest
 preceding the three write statements with a comment along the lines of "We
 know that srvid is at least 8 bytes in length, so...".  This makes the
 code marginally faster.

 (Note that the assumption that there is enough space in a buffer is made
 in unpackOptions(), where an explicit length is passed to the readUint16()
 calls. In that case, the assumption is made in the "while" condition in
 that the readUint16() functions won't be executed if there is less than
 four bytes remaining.)


 '''src/lib/asiodns/tests/io_fetch_unittest.cc'''[[BR]]
 Although "sizeof" is a keyword, the general usage elsewhere in BIND 10
 appears to be treat it as a function.  So the "standard" appears to write
 it as "sizeof(xxx)". not "sizeof (xxxgdif )".  The embedded space should
 be removed.


 '''src/lib/asiolink/tests/tcp_socket_unittest.cc'''[[BR]]
 See comment for io_fetch_unittest.cc

 !SequenceTest: to be really generic, TCPCallback::MIN_SIZE could be used
 as the size argument for writeUint16(), as that is the size to which the
 data buffer returned by server_cb.data() is set.

 '''src/lib/asiolink/tests/udp_socket_unittest.cc'''[[BR]]
 See comment for io_fetch_unittest.cc

 '''src/lib/dhcp/option.cc'''[[BR]]
 As readUintNN() now checks the size and throws the !OutOfRange exception
 if the size is invalid, the equivalent checks in Option::getUintNN() can
 be removed.

 Although not strictly part of this change, in the setUintNN() functions
 the argument to the data_.resize() function call could be a
 "sizeof(datatype)" instead of an explicit literal.

 '''src/lib/dhcp/option4_addrlst.cc'''[[BR]]
 '''src/lib/dhcp/option6_ia.cc'''[[BR]]
 '''src/lib/dhcp/option6_iaaddr.cc'''[[BR]]
 '''src/lib/dhcp/option6_iaprefix.cc'''[[BR]]
 '''src/lib/dhcp/option_int.h'''[[BR]]
 '''src/lib/dhcp/option_int_array.h'''[[BR]]
 '''src/lib/dhcp/option_vendor.cc'''[[BR]]
 I'm not sure that use of "distance" is strictly correct here.  The
 readUintNN() calls require as argument the number of bytes in the buffer,
 but "distance" returns the number of elements (which may not be equal to
 the number of bytes) between two pointers. Use of "distance" relies on
 !OptionBufferConstIterator being a pointer to a byte. However, as most of
 the code here in these files make that (valid) assumption, I don't think
 it is worth changing.


 '''src/lib/resolve/tests/recursive_query_unittest_2.cc'''[[BR]]
 '''src/lib/resolve/tests/recursive_query_unittest_3.cc'''[[BR]]
 '''src/lib/util/io_utilities.h'''[[BR]]
 '''src/lib/util/tests/io_utilities_unittest.cc'''[[BR]]
 See comment for io_fetch_unittest.cc

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


More information about the bind10-tickets mailing list