From: Marcin Siodelski <marcin@isc.org>
Date: Thu, 22 May 2014 17:00:45 +0000 (+0200)
Subject: [3246] Addressed review comments.
X-Git-Tag: trac3434_base~26^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=dc9a95754e8b5b8a8d67c5ac93a6acb915aa1345;p=thirdparty%2Fkea.git

[3246] Addressed review comments.
---

diff --git a/src/lib/dhcp/option6_iaprefix.cc b/src/lib/dhcp/option6_iaprefix.cc
index 43de8fe8b0..5aded2e90f 100644
--- a/src/lib/dhcp/option6_iaprefix.cc
+++ b/src/lib/dhcp/option6_iaprefix.cc
@@ -133,8 +133,18 @@ Option6IAPrefix::mask(OptionBuffer::const_iterator begin,
     if (len >= 128) {
         std::copy(begin, end, output_address.begin());
     } else if (len > 0) {
+        // All the bits that represent whole octets of the prefix are copied with
+        // no change.
         std::copy(begin, begin + static_cast<uint8_t>(len/8), output_address.begin());
-        output_address[len/8] = (0xff << (8 - (len % 8)));
+        // The remaining significant bits of the last octet have to be left unchanged,
+        // but the remaining bits of this octet must be set to zero. The number of
+        // significant bits is calculated as a reminder from the devision of the
+        // prefix length by 8 (by size of the octet). The number of bits to be set
+        // to zero is therefore calculated as: 8 - (len % 8).
+        // Next, the mask is created by shifting the 0xFF by the number of bits
+        // to be set to 0. By performing logical AND of this mask with the original
+        // value of the last octet we get the final value for the new octet.
+        output_address[len/8] = (*(begin + len/8) & (0xFF << (8 - (len % 8))));
     }
 }
 
diff --git a/src/lib/dhcp/option6_iaprefix.h b/src/lib/dhcp/option6_iaprefix.h
index f80e4748c7..1ffb9a2df7 100644
--- a/src/lib/dhcp/option6_iaprefix.h
+++ b/src/lib/dhcp/option6_iaprefix.h
@@ -94,6 +94,11 @@ public:
 
     /// @brief Parses received buffer.
     ///
+    /// This function calls the @c Option6IAPrefix::mask function to set the
+    /// non-significant bits of the prefix (bits beyond the length of the
+    /// prefix) to zero. See the @c Option6IAPrefix class documentation for
+    /// details why it is done.
+    ///
     /// @throw OutOfRange when buffer is shorter than 25 bytes
     ///
     /// @param begin iterator to first byte of option data
diff --git a/src/lib/dhcp/tests/option6_iaprefix_unittest.cc b/src/lib/dhcp/tests/option6_iaprefix_unittest.cc
index 319d4dc072..892f40b2fb 100644
--- a/src/lib/dhcp/tests/option6_iaprefix_unittest.cc
+++ b/src/lib/dhcp/tests/option6_iaprefix_unittest.cc
@@ -45,7 +45,7 @@ public:
     /// @brief creates on-wire representation of IAPREFIX option
     ///
     /// buf_ field is set up to have IAPREFIX with preferred=1000,
-    /// valid=3000000000 and prefix being 2001:db8:1:0:ffff:0:dead:beef/77
+    /// valid=3000000000 and prefix being 2001:db8:1:0:afaf:0:dead:beef/77
     void setExampleBuffer() {
         for (int i = 0; i < 255; i++) {
             buf_[i] = 0;
@@ -69,12 +69,12 @@ public:
         buf_[12] = 0xb8;
         buf_[13] = 0x00;
         buf_[14] = 0x01;
-        buf_[17] = 0xff;
-        buf_[18] = 0xff;
+        buf_[17] = 0xaf;
+        buf_[18] = 0xaf;
         buf_[21] = 0xde;
         buf_[22] = 0xad;
         buf_[23] = 0xbe;
-        buf_[24] = 0xef; // 2001:db8:1:0:ffff:0:dead:beef
+        buf_[24] = 0xef; // 2001:db8:1:0:afaf:0:dead:beef
     }
 
 
@@ -148,11 +148,11 @@ TEST_F(Option6IAPrefixTest, parseShort) {
 
     // The non-significant bits (above 77) of the received prefix should be
     // set to zero.
-    checkOption(*opt, D6O_IAPREFIX, 77, "2001:db8:1:0:fff8::");
+    checkOption(*opt, D6O_IAPREFIX, 77, "2001:db8:1:0:afa8::");
 
     // Set non-significant bits in the reference buffer to 0, so as the buffer
     // can be directly compared with the option buffer.
-    buf_[18] = 0xf8;
+    buf_[18] = 0xa8;
     buf_.insert(buf_.begin() + 19, 5, 0);
     checkOutputBuffer(D6O_IAPREFIX);
 
@@ -177,7 +177,7 @@ TEST_F(Option6IAPrefixTest, parseLong) {
     opt->pack(out_buf_);
     EXPECT_EQ(29, out_buf_.getLength());
 
-    checkOption(*opt, D6O_IAPREFIX, 128, "2001:db8:1:0:ffff:0:dead:beef");
+    checkOption(*opt, D6O_IAPREFIX, 128, "2001:db8:1:0:afaf:0:dead:beef");
 
     checkOutputBuffer(D6O_IAPREFIX);
 
@@ -219,11 +219,11 @@ TEST_F(Option6IAPrefixTest, build) {
     setExampleBuffer();
 
     ASSERT_NO_THROW(opt.reset(new Option6IAPrefix(12345,
-                    IOAddress("2001:db8:1:0:ffff:0:dead:beef"), 77,
+                    IOAddress("2001:db8:1:0:afaf:0:dead:beef"), 77,
                                                   1000, 3000000000u)));
     ASSERT_TRUE(opt);
 
-    checkOption(*opt, 12345, 77, "2001:db8:1:0:ffff:0:dead:beef");
+    checkOption(*opt, 12345, 77, "2001:db8:1:0:afaf:0:dead:beef");
 
     // Check if we can build it properly
     EXPECT_NO_THROW(opt->pack(out_buf_));