BIND 10 trac1278, updated. 0b5da8bd0800bfa3744e23c367cee2c38de7a497 [1278] address review comments

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Nov 30 16:00:00 UTC 2011


The branch, trac1278 has been updated
       via  0b5da8bd0800bfa3744e23c367cee2c38de7a497 (commit)
      from  eb6053d466fcea08fa66205d598d316e550863c8 (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 0b5da8bd0800bfa3744e23c367cee2c38de7a497
Author: Jelte Jansen <jelte at isc.org>
Date:   Wed Nov 30 16:59:50 2011 +0100

    [1278] address review comments

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

Summary of changes:
 src/lib/dns/python/serial_python.cc            |   10 ++++------
 src/lib/dns/python/tests/serial_python_test.py |    5 +++++
 src/lib/dns/serial.cc                          |   22 +++++-----------------
 src/lib/dns/serial.h                           |    9 ++++++---
 src/lib/dns/tests/serial_unittest.cc           |   10 +++++-----
 5 files changed, 25 insertions(+), 31 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dns/python/serial_python.cc b/src/lib/dns/python/serial_python.cc
index 2f9501c..e2bd809 100644
--- a/src/lib/dns/python/serial_python.cc
+++ b/src/lib/dns/python/serial_python.cc
@@ -144,8 +144,7 @@ Serial_richcmp(s_Serial* self, s_Serial* other, int op) {
         c = *self->cppobj < *other->cppobj;
         break;
     case Py_LE:
-        c = *self->cppobj < *other->cppobj ||
-            *self->cppobj == *other->cppobj;
+        c = *self->cppobj <= *other->cppobj;
         break;
     case Py_EQ:
         c = *self->cppobj == *other->cppobj;
@@ -154,11 +153,10 @@ Serial_richcmp(s_Serial* self, s_Serial* other, int op) {
         c = *self->cppobj != *other->cppobj;
         break;
     case Py_GT:
-        c = *other->cppobj < *self->cppobj;
+        c = *self->cppobj > *other->cppobj;
         break;
     case Py_GE:
-        c = *other->cppobj < *self->cppobj ||
-            *self->cppobj == *other->cppobj;
+        c = *self->cppobj >= *other->cppobj;
         break;
     }
     if (c) {
@@ -222,7 +220,7 @@ PyTypeObject serial_type = {
     "main purpose of this class is to provide serial number arithmetic, as "
     "described in RFC 1892. Objects of this type can be compared and added "
     "to each other, as described in RFC 1892. Apart from str(), get_value(), "
-    "comparison operators, and the + operand, no other operations are "
+    "comparison operators, and the + operator, no other operations are "
     "defined for this type.",
     NULL,                               // tp_traverse
     NULL,                               // tp_clear
diff --git a/src/lib/dns/python/tests/serial_python_test.py b/src/lib/dns/python/tests/serial_python_test.py
index 01f999e..4e87f3d 100644
--- a/src/lib/dns/python/tests/serial_python_test.py
+++ b/src/lib/dns/python/tests/serial_python_test.py
@@ -100,5 +100,10 @@ class SerialTest(unittest.TestCase):
         self.assertEqual(100 + self.one, self.highest + 102)
         self.assertEqual(self.zero + 2147483645, self.highest + 2147483646)
 
+        # using lambda so the error doesn't get thrown on initial evaluation
+        self.assertRaises(TypeError, lambda: self.zero + "bad")
+        self.assertRaises(TypeError, lambda: self.zero + None)
+        self.assertRaises(TypeError, lambda: "bad" + self.zero)
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/src/lib/dns/serial.cc b/src/lib/dns/serial.cc
index 3623f67..7f07444 100644
--- a/src/lib/dns/serial.cc
+++ b/src/lib/dns/serial.cc
@@ -32,9 +32,9 @@ Serial::operator<(const Serial& other) const {
     uint32_t other_val = other.getValue();
     bool result = false;
     if (value_ < other_val) {
-        result = ((other_val - value_) <= MAX_INCREMENT);
+        result = ((other_val - value_) <= MAX_SERIAL_INCREMENT);
     } else if (other_val < value_) {
-        result = ((value_ - other_val) > MAX_INCREMENT);
+        result = ((value_ - other_val) > MAX_SERIAL_INCREMENT);
     }
     return (result);
 }
@@ -56,21 +56,9 @@ Serial::operator>=(const Serial& other) const {
 
 Serial
 Serial::operator+(uint32_t other_val) const {
-    if (value_ <= MAX_INCREMENT) {
-        // just add
-        return (Serial(value_ + other_val));
-    } else {
-        // check whether it wouldn't wrap
-        // Note the +1 here and the -2 below, these operations use
-        // the MAX_INCREMENT constant, which is 2^31-1, while it really
-        // needs 2^31 itself.
-        if (value_ - MAX_INCREMENT + other_val <= MAX_INCREMENT + 1) {
-            return (Serial(value_ + other_val));
-        } else {
-            return (Serial(value_ - MAX_INCREMENT +
-                           other_val - MAX_INCREMENT - 2));
-        }
-    }
+    uint64_t new_val = static_cast<uint64_t>(value_) +
+                       static_cast<uint64_t>(other_val);
+    return Serial(static_cast<uint32_t>(new_val % MAX_SERIAL_VALUE));
 }
 
 Serial
diff --git a/src/lib/dns/serial.h b/src/lib/dns/serial.h
index 9a2fdae..620e274 100644
--- a/src/lib/dns/serial.h
+++ b/src/lib/dns/serial.h
@@ -24,7 +24,10 @@ namespace dns {
 /// The maximum difference between two serial numbers. If the (plain uint32_t)
 /// difference between two serials is greater than this number, the smaller one
 /// is considered greater.
-const uint32_t MAX_INCREMENT = 2147483647;
+const uint32_t MAX_SERIAL_INCREMENT = 2147483647;
+
+/// Maximum value a serial can have, used in + operator.
+const uint64_t MAX_SERIAL_VALUE = 4294967296;
 
 /// \brief This class defines DNS serial numbers and serial arithmetic.
 ///
@@ -115,7 +118,7 @@ public:
     /// \brief Adds the given value to the serial number. If this would make
     /// the number greater than 2^32-1, it is 'wrapped'.
     /// \note According to the specification, an addition greater than
-    /// MAX_INCREMENT is undefined. We do NOT catch this error (so as not
+    /// MAX_SERIAL_INCREMENT is undefined. We do NOT catch this error (so as not
     /// to raise exceptions), but this behaviour remains undefined.
     ///
     /// \param other The Serial to add
@@ -127,7 +130,7 @@ public:
     /// the number greater than 2^32-1, it is 'wrapped'.
     ///
     /// \note According to the specification, an addition greater than
-    /// MAX_INCREMENT is undefined. We do NOT catch this error (so as not
+    /// MAX_SERIAL_INCREMENT is undefined. We do NOT catch this error (so as not
     /// to raise exceptions), but this behaviour remains undefined.
     ///
     /// \param other_val The uint32_t value to add
diff --git a/src/lib/dns/tests/serial_unittest.cc b/src/lib/dns/tests/serial_unittest.cc
index da2d753..128c92d 100644
--- a/src/lib/dns/tests/serial_unittest.cc
+++ b/src/lib/dns/tests/serial_unittest.cc
@@ -88,7 +88,7 @@ TEST_F(SerialTest, addition) {
 
     EXPECT_EQ(one + 100, max + 102);
     EXPECT_EQ(min + 2147483645, max + 2147483646);
-    EXPECT_EQ(min + 2147483646, max + MAX_INCREMENT);
+    EXPECT_EQ(min + 2147483646, max + MAX_SERIAL_INCREMENT);
 }
 
 //
@@ -109,8 +109,8 @@ void do_addition_larger_test(const Serial& number) {
     EXPECT_GT(number + 100, number);
     EXPECT_GT(number + 1111111, number);
     EXPECT_GT(number + 2147483646, number);
-    EXPECT_GT(number + MAX_INCREMENT, number);
-    // Try MAX_INCREMENT as a hardcoded number as well
+    EXPECT_GT(number + MAX_SERIAL_INCREMENT, number);
+    // Try MAX_SERIAL_INCREMENT as a hardcoded number as well
     EXPECT_GT(number + 2147483647, number);
 }
 
@@ -142,7 +142,7 @@ do_two_additions_test_second(const Serial &original,
     EXPECT_NE(original, number + 100);
     EXPECT_NE(original, number + 1111111);
     EXPECT_NE(original, number + 2147483646);
-    EXPECT_NE(original, number + MAX_INCREMENT);
+    EXPECT_NE(original, number + MAX_SERIAL_INCREMENT);
     EXPECT_NE(original, number + 2147483647);
 }
 
@@ -152,7 +152,7 @@ void do_two_additions_test_first(const Serial &number) {
     do_two_additions_test_second(number, number + 100);
     do_two_additions_test_second(number, number + 1111111);
     do_two_additions_test_second(number, number + 2147483646);
-    do_two_additions_test_second(number, number + MAX_INCREMENT);
+    do_two_additions_test_second(number, number + MAX_SERIAL_INCREMENT);
     do_two_additions_test_second(number, number + 2147483647);
 }
 




More information about the bind10-changes mailing list