BIND 10 #1313: writeUint32 and readUint32 are needed

[BIND 10 Development]

#1313: writeUint32 and readUint32 are needed
-------------------------------------+-------------------------------------
                   Reporter:  tomek  |                 Owner:  tomek
                       Type:         |                Status:  reviewing
  enhancement                        |             Milestone:  DHCP 2011
                   Priority:  major  |            Resolution:
                  Component:  dhcp   |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DHCP
            Defect Severity:  N/A    |  Estimated Difficulty:  0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  UnAssigned => tomek


Comment:

 Responding to [comment:3 jinmei]:

 > We have much safer abstraction of buffers:
 isc::util::!Input/OutputBuffer. I strongly suggest using the safer
 interfaces.
 The original read/writeUintXX were for tests which, on reflection, could
 probably be implemented without it.

 !Input/OutputBuffer is being used for new code and it is likely that the
 existing code will be refactored in this way.

 > And I actually plan to open a ticket to deprecate the dangerous ones
 Dangerous is probably too strong a word - an unexploded bomb is dangerous,
 this interface is perhaps just unsafe? :-)

 As to the review:

 '''src/lib/util/io_utilities.h'''[[BR]]
 Header comments are wrong - the buffer is assumed to be four bytes long,
 not two bytes.


 '''src/lib/util/tests/io_utilities_unittest.cc'''[[BR]]
 Should be spaces around binary operators (e.g. "offset < 4" instead of
 "offset<4").

 Should be no spaces (except in exceptional circumstances) after a "(" and
 before a ")".

 readUint32 test: the index of the inner loop (i) ranges between 0 and
 sizeof(test32).  However, i is then used as an index into test32.  As
 sizeof(test32) is in bytes, and test32 is a uint32_t array, this means
 that for values of i > 4, the loop is accessing elements beyond the end of
 the array.  The end value of the inner loop should be {{{sizeof(test32) /
 sizeof(uint32_t)}}}.

 The same comment applies for the writeUint32 test.

 writeUint32 test: suggest the final assertion be written as:
 {{{
 EXPECT_EQ(0, memcmp(...))
 }}}
 Although I know that 0 tests as false in C/C++, the impression of the line
 containing the EXPECT_FALSE is that the memory comparison is expected to
 fail.

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