Merge branch 'trac1313'

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 four bytes long of which the first four
+///        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 four bytes long into which the 32-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

src/lib/util/tests/io_utilities_unittest.cc

@@ -19,6 +19,7 @@
 
 #include <cstddef>
 
+#include <arpa/inet.h>
 #include <gtest/gtest.h>
 
 #include <util/buffer.h>
@@ -71,3 +72,48 @@ TEST(asioutil, writeUint16) {
         EXPECT_EQ(ref[1], test[1]);
     }
 }
+
+// test data shared amount readUint32 and writeUint32 tests
+const static uint32_t test32[] = {
+    0,
+    1,
+    2000,
+    0x80000000,
+    0xffffffff
+};
+
+TEST(asioutil, readUint32) {
+    uint8_t data[8];
+
+    // make sure that we can read data, regardless of
+    // the memory alignment. That' why we need to repeat
+    // it 4 times.
+    for (int offset=0; offset < 4; offset++) {
+        for (int i=0; i < sizeof(test32)/sizeof(uint32_t); i++) {
+            uint32_t tmp = htonl(test32[i]);
+            memcpy(&data[offset], &tmp, sizeof(uint32_t));
+
+            EXPECT_EQ(test32[i], readUint32(&data[offset]));
+        }
+    }
+}
+
+
+TEST(asioutil, writeUint32) {
+    uint8_t data[8];
+
+    // make sure that we can write data, regardless of
+    // the memory alignment. That's why we need to repeat
+    // it 4 times.
+    for (int offset=0; offset < 4; offset++) {
+        for (int i=0; i < sizeof(test32)/sizeof(uint32_t); i++) {
+            uint8_t* ptr = writeUint32(test32[i], &data[offset]);
+
+            EXPECT_EQ(&data[offset]+sizeof(uint32_t), ptr);
+
+            uint32_t tmp = htonl(test32[i]);
+
+            EXPECT_EQ(0, memcmp(&tmp, &data[offset], sizeof(uint32_t)));
+        }
+    }
+}