[1313] writeUint32, readUint32 implemented

src/lib/dhcp/option.cc

@@ -86,19 +86,17 @@ Option::pack6(boost::shared_array<uint8_t>& buf,
                   type_ << ",len=" << len() << ": too small buffer.");
     }
 
-    int length = len() - getHeaderLen();
-
     uint8_t * ptr = &buf[offset];
-    writeUint16(type_, ptr);
-    ptr += sizeof(uint16_t);
 
-    writeUint16(length, ptr);
-    ptr += sizeof(uint16_t);
+    ptr = writeUint16(type_, ptr);
+
+    ptr = writeUint16(len() - getHeaderLen(), ptr);
 
     if (data_len_)
         memcpy(ptr, &data_[offset_], data_len_);
 
-    offset += OPTION6_HDR_LEN + data_len_; // end of this option
+    // end of fixed part of this option
+    offset += OPTION6_HDR_LEN + data_len_;
 
     return LibDHCP::packOptions6(buf, buf_len, offset, options_);
 }

src/lib/dhcp/option6_ia.cc

@@ -54,25 +54,18 @@ Option6IA::pack(boost::shared_array<uint8_t>& buf,
                   << len() << " is too small (at least 16 is required).");
     }
 
-    writeUint16(type_, &buf[offset]);
-    offset += sizeof(uint16_t);
-
-    writeUint16(len() - OPTION6_HDR_LEN, &buf[offset]);
-    offset += sizeof(uint16_t);
-
-    /// TODO start using writeUint32 once such function is implemented
     uint8_t* ptr = &buf[offset];
 
-    *(uint32_t*)ptr = htonl(iaid_);
-    ptr += sizeof(uint32_t);
+    ptr = writeUint16(type_, ptr);
+    ptr = writeUint16(len() - OPTION6_HDR_LEN, ptr);
+    offset += OPTION6_HDR_LEN;
 
-    *(uint32_t*)ptr = htonl(t1_);
-    ptr += sizeof(uint32_t);
+    ptr = writeUint32(iaid_, ptr);
+    ptr = writeUint32(t1_, ptr);
+    ptr = writeUint32(t2_, ptr);
+    offset += OPTION6_IA_LEN;
 
-    *(uint32_t*)ptr = htonl(t2_);
-    ptr += sizeof(uint32_t);
-
-    offset = LibDHCP::packOptions6(buf, buf_len, offset+12, options_);
+    offset = LibDHCP::packOptions6(buf, buf_len, offset, options_);
     return offset;
 }
 
@@ -84,16 +77,16 @@ Option6IA::unpack(const boost::shared_array<uint8_t>& buf,
     if ( parse_len < OPTION6_IA_LEN || offset + OPTION6_IA_LEN > buf_len) {
         isc_throw(OutOfRange, "Option " << type_ << " truncated");
     }
-
-    /// TODO this will cause SIGBUS on sparc if we happen to read misaligned
-    /// memory access. We need to fix this (and similar code) as part of
-    /// the ticket #1313
-    iaid_ = ntohl(*(uint32_t*)&buf[offset]);
+    
+    iaid_ = readUint32(&buf[offset]);
     offset += sizeof(uint32_t);
-    t1_ = ntohl(*(uint32_t*)&buf[offset]);
+
+    t1_ = readUint32(&buf[offset]);
     offset += sizeof(uint32_t);
-    t2_ = ntohl(*(uint32_t*)&buf[offset]);
+
+    t2_ = readUint32(&buf[offset]);
     offset += sizeof(uint32_t);
+
     offset = LibDHCP::unpackOptions6(buf, buf_len, offset,
                                      parse_len - OPTION6_IA_LEN, options_);
 

src/lib/dhcp/option6_iaaddr.cc

@@ -53,19 +53,22 @@ Option6IAAddr::pack(boost::shared_array<uint8_t>& buf,
                   << ", buffer=" << buf_len << ": too small buffer.");
     }
 
-    *(uint16_t*)&buf[offset] = htons(type_);
-    offset += sizeof(uint16_t);
-    *(uint16_t*)&buf[offset] = htons(len() - OPTION6_HDR_LEN); // len() returns complete option
-    // length. len field contains length without 4-byte option header
-    offset += sizeof(uint16_t);
+    uint8_t* ptr = &buf[offset];
 
-    memcpy(&buf[offset], addr_.getAddress().to_v6().to_bytes().data(), 16);
-    offset += V6ADDRESS_LEN;
+    ptr = writeUint16(type_, ptr);
 
-    *(uint32_t*)&buf[offset] = htonl(preferred_);
-    offset += sizeof(uint32_t);
-    *(uint32_t*)&buf[offset] = htonl(valid_);
-    offset += sizeof(uint32_t);
+    // len() returns complete option length. len field contains
+    // length without 4-byte option header
+    ptr = writeUint16(len() - OPTION6_HDR_LEN, ptr);
+    offset += OPTION6_HDR_LEN;
+
+    memcpy(ptr, addr_.getAddress().to_v6().to_bytes().data(), 16);
+    ptr += V6ADDRESS_LEN;
+
+    ptr = writeUint32(preferred_, ptr);
+
+    ptr = writeUint32(valid_, ptr);
+    offset += OPTION6_IAADDR_LEN;
 
     // parse suboption (there shouldn't be any)
     offset = LibDHCP::packOptions6(buf, buf_len, offset, options_);
@@ -85,10 +88,10 @@ Option6IAAddr::unpack(const boost::shared_array<uint8_t>& buf,
     addr_ = IOAddress::from_bytes(AF_INET6, &buf[offset]);
     offset += V6ADDRESS_LEN;
 
-    preferred_ = ntohl(*(uint32_t*)&buf[offset]);
+    preferred_ = readUint32(&buf[offset]);
     offset += sizeof(uint32_t);
 
-    valid_ = ntohl(*(uint32_t*)&buf[offset]);
+    valid_ = readUint32(&buf[offset]);
     offset += sizeof(uint32_t);
     offset = LibDHCP::unpackOptions6(buf, buf_len, offset,
                                      parse_len - 24, options_);

src/lib/dhcp/tests/option6_ia_unittest.cc

@@ -132,8 +132,7 @@ TEST_F(Option6IATest, simple) {
 }
 
 // test if option can build suboptions
-/// TODO Reenable once ticket #1313 is implemented
-TEST_F(Option6IATest, DISABLED_suboptions_pack) {
+TEST_F(Option6IATest, suboptions_pack) {
     boost::shared_array<uint8_t> buf(new uint8_t[128]);
     for (int i=0; i<128; i++)
         buf[i] = 0;

src/lib/dhcp/tests/option6_iaaddr_unittest.cc

@@ -35,11 +35,12 @@ public:
 };
 
 /// TODO reenable this once ticket #1313 is implemented.
-TEST_F(Option6IAAddrTest, DISABLED_basic) {
+TEST_F(Option6IAAddrTest, basic) {
 
     boost::shared_array<uint8_t> simple_buf(new uint8_t[128]);
     for (int i = 0; i < 128; i++)
         simple_buf[i] = 0;
+
     simple_buf[0] = 0x20;
     simple_buf[1] = 0x01;
     simple_buf[2] = 0x0d;

src/lib/dhcp/tests/pkt6_unittest.cc

@@ -85,8 +85,7 @@ Pkt6 *capture1() {
     return (pkt);
 }
 
-/// TODO Reenable this once ticket #1313 is implemented
-TEST_F(Pkt6Test, DISABLED_unpack_solicit1) {
+TEST_F(Pkt6Test, unpack_solicit1) {
     Pkt6 * sol = capture1();
 
     ASSERT_EQ(true, sol->unpack());

src/lib/util/io_utilities.h

@@ -48,13 +48,54 @@ readUint16(const void* buffer) {
 /// \param value 16-bit value to convert
 /// \param buffer Data buffer at least two bytes long into which the 16-bit
 ///        value is written in network-byte order.
-
-inline void
+///
+/// \return pointer to the next byte after stored value
+inline uint8_t*
 writeUint16(uint16_t value, void* buffer) {
     uint8_t* byte_buffer = static_cast<uint8_t*>(buffer);
 
     byte_buffer[0] = static_cast<uint8_t>((value & 0xff00U) >> 8);
     byte_buffer[1] = static_cast<uint8_t>(value & 0x00ffU);
+
+    return (byte_buffer + sizeof(uint16_t));
+}
+
+/// \brief Read Unsigned 32-Bit Integer from Buffer
+///
+/// \param buffer Data buffer at least two bytes long of which the first two
+///        bytes are assumed to represent a 32-bit integer in network-byte
+///        order.
+///
+/// \return Value of 32-bit unsigned integer
+inline uint32_t
+readUint32(const uint8_t* buffer) {
+    const uint8_t* byte_buffer = static_cast<const uint8_t*>(buffer);
+
+    uint32_t result = (static_cast<uint32_t>(byte_buffer[0])) << 24;
+    result |= (static_cast<uint32_t>(byte_buffer[1])) << 16;
+    result |= (static_cast<uint32_t>(byte_buffer[2])) << 8;
+    result |= (static_cast<uint32_t>(byte_buffer[3]));
+
+    return (result);
+}
+
+/// \brief Write Unisgned 32-Bit Integer to Buffer
+///
+/// \param value 32-bit value to convert
+/// \param buffer Data buffer at least two bytes long into which the 16-bit
+///        value is written in network-byte order.
+///
+/// \return pointer to the next byte after stored value
+inline uint8_t*
+writeUint32(uint32_t value, uint8_t* buffer) {
+    uint8_t* byte_buffer = static_cast<uint8_t*>(buffer);
+
+    byte_buffer[0] = static_cast<uint8_t>((value & 0xff000000U) >> 24);
+    byte_buffer[1] = static_cast<uint8_t>((value & 0x00ff0000U) >> 16);
+    byte_buffer[2] = static_cast<uint8_t>((value & 0x0000ff00U) >>  8);
+    byte_buffer[3] = static_cast<uint8_t>((value & 0x000000ffU));
+
+    return (byte_buffer + sizeof(uint32_t));
 }
 
 } // namespace util