BIND 10 trac1228, updated. 5d6c71aeb2575883488b2cde87501aa84260b1ab [1228] Changes after review.

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Nov 4 11:40:10 UTC 2011


The branch, trac1228 has been updated
       via  5d6c71aeb2575883488b2cde87501aa84260b1ab (commit)
      from  05d4deb643271e0f0b0dcfb22809714086d50788 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 5d6c71aeb2575883488b2cde87501aa84260b1ab
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Fri Nov 4 12:39:15 2011 +0100

    [1228] Changes after review.
    
    - check() method added in Option class. Constructors are now simpler.
    - compilation fix in option_unittest.cc

-----------------------------------------------------------------------

Summary of changes:
 src/lib/dhcp/option.cc                |   54 ++++++++++++++++----------------
 src/lib/dhcp/option.h                 |   11 +++++--
 src/lib/dhcp/tests/option_unittest.cc |    4 +-
 3 files changed, 37 insertions(+), 32 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/option.cc b/src/lib/dhcp/option.cc
index 546805d..daef288 100644
--- a/src/lib/dhcp/option.cc
+++ b/src/lib/dhcp/option.cc
@@ -43,46 +43,46 @@ Option::Option(Universe u, unsigned short type,
     :universe_(u), type_(type),
      offset_(offset)
 {
-    if ( (u == V4) && (type > 255)) {
-        isc_throw(BadValue, "Can't create V4 option of type "
-                  << type << ", V4 options are in range 0..255");
-    }
-
     uint8_t* ptr = &buf[offset];
     data_ = std::vector<uint8_t>(ptr, ptr + len);
 
-    // sanity checks
-    // TODO: universe must be in V4 and V6
+    check();
 }
 
 Option::Option(Universe u, unsigned short type, std::vector<uint8_t>& data)
     :universe_(u), type_(type), data_(data) {
-    if ( (u == V4) && (data.size() > 255) ) {
-        isc_throw(OutOfRange, "DHCPv4 Option " << type_ << " is too big."
-                  << "At most 255 bytes are supported.");
-        /// TODO Larger options can be stored as separate instances
-        /// of DHCPv4 options. Clients MUST concatenate them.
-        /// Fortunately, there are no such large options used today.
-    }
-    if ( (u == V4) && (type > 255) ) {
-        isc_throw(OutOfRange, "DHCPv4 Option type " << type_ << " is too big."
-                  << "For DHCPv4 allowed type range is 0..255");
-    }
+    check();
 }
 
 Option::Option(Universe u, uint16_t type, vector<uint8_t>::const_iterator first,
                vector<uint8_t>::const_iterator last)
-    :universe_(u), type_(type) {
-    if ( (u == V4) && (type > 255) ) {
-        isc_throw(OutOfRange, "DHCPv4 Option type " << type_ << " is too big."
-                  << "For DHCPv4 allowed type range is 0..255");
+    :universe_(u), type_(type), data_(std::vector<uint8_t>(first,last)) {
+    check();
+}
+
+void
+Option::check() {
+    if ( (universe_ != V4) && (universe_ != V6) ) {
+        isc_throw(BadValue, "Invalid universe type specified."
+                  << "Only V4 and V6 are allowed.");
     }
-    data_ = std::vector<uint8_t>(first, last);
-    if ( (u == V4) && (data_.size() > 255) ) {
-        isc_throw(OutOfRange, "DHCPv4 Option " << type_ << " is too big.");
+
+    if (universe_ == V4) {
+
+        if (type_ > 255) {
+            isc_throw(OutOfRange, "DHCPv4 Option type " << type_ << " is too big."
+                      << "For DHCPv4 allowed type range is 0..255");
+        } else if (data_.size() > 255) {
+            isc_throw(OutOfRange, "DHCPv4 Option " << type_ << " is too big.");
+            /// TODO Larger options can be stored as separate instances
+            /// of DHCPv4 options. Clients MUST concatenate them.
+            /// Fortunately, there are no such large options used today.
+        }
     }
-}
 
+    // no need to check anything for DHCPv6. It allows full range (0-64k) of
+    // both types and data size.
+}
 
 unsigned int
 Option::pack(boost::shared_array<uint8_t>& buf,
@@ -153,7 +153,7 @@ Option::pack6(boost::shared_array<uint8_t>& buf,
                   type_ << ",len=" << len() << ": too small buffer.");
     }
 
-    uint8_t * ptr = &buf[offset];
+    uint8_t* ptr = &buf[offset];
 
     ptr = writeUint16(type_, ptr);
 
diff --git a/src/lib/dhcp/option.h b/src/lib/dhcp/option.h
index ac4544b..3822cf0 100644
--- a/src/lib/dhcp/option.h
+++ b/src/lib/dhcp/option.h
@@ -90,7 +90,6 @@ public:
     /// @param data content of the option
     Option(Universe u, unsigned short type, std::vector<uint8_t>& data);
 
-
     /// @brief Constructor, used for received options.
     ///
     /// This contructor is similar to the previous one, but it does not take
@@ -136,8 +135,7 @@ public:
     /// @return offset to first unused byte after stored option
     ///
     virtual unsigned int
-    pack(boost::shared_array<uint8_t>& buf,
-         unsigned int buf_len,
+    pack(boost::shared_array<uint8_t>& buf, unsigned int buf_len,
          unsigned int offset);
 
     /// @brief Writes option in a wire-format to a buffer.
@@ -297,6 +295,13 @@ protected:
             unsigned int offset,
             unsigned int parse_len);
 
+    /// @brief A private method used for option correctness.
+    ///
+    /// It is used in constructors. In there are any problems detected
+    /// (like specifying type > 255 for DHCPv4 option), it will throw
+    /// BadValue or OutOfRange exceptions.
+    void check();
+
     /// option universe (V4 or V6)
     Universe universe_;
 
diff --git a/src/lib/dhcp/tests/option_unittest.cc b/src/lib/dhcp/tests/option_unittest.cc
index 1e28c7c..db3ee3b 100644
--- a/src/lib/dhcp/tests/option_unittest.cc
+++ b/src/lib/dhcp/tests/option_unittest.cc
@@ -87,7 +87,7 @@ TEST_F(OptionTest, v4_data1) {
     EXPECT_EQ(123, opt->getType());
     vector<uint8_t> optData = opt->getData();
     ASSERT_EQ(optData.size(), data.size());
-    EXPECT_EQ(optData, data);
+    EXPECT_TRUE(optData == data);
     EXPECT_EQ(2, opt->getHeaderLen());
     EXPECT_EQ(6, opt->len());
 
@@ -147,7 +147,7 @@ TEST_F(OptionTest, v4_data2) {
     EXPECT_EQ(123, opt->getType());
     vector<uint8_t> optData = opt->getData();
     ASSERT_EQ(optData.size(), expData.size());
-    EXPECT_EQ(optData, expData);
+    EXPECT_TRUE(optData == expData);
     EXPECT_EQ(2, opt->getHeaderLen());
     EXPECT_EQ(6, opt->len());
 




More information about the bind10-changes mailing list