BIND 10 trac1959, updated. b5aa1dadff2c0e7c9fce8fab0aa6dee28071e29b [1959] Implemented suggestions from code review.

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Sep 10 18:44:07 UTC 2012


The branch, trac1959 has been updated
       via  b5aa1dadff2c0e7c9fce8fab0aa6dee28071e29b (commit)
      from  10cf1a64da6a0d8851984e6ca0c6c7baa6769c20 (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 b5aa1dadff2c0e7c9fce8fab0aa6dee28071e29b
Author: Marcin Siodelski <marcin at isc.org>
Date:   Mon Sep 10 20:43:51 2012 +0200

    [1959] Implemented suggestions from code review.

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

Summary of changes:
 tests/tools/perfdhcp/command_options.cc            |    3 +-
 tests/tools/perfdhcp/pkt_transform.h               |    2 ++
 tests/tools/perfdhcp/test_control.cc               |   36 ++++++++++++--------
 tests/tools/perfdhcp/test_control.h                |   34 +++++++++++-------
 .../tools/perfdhcp/tests/test_control_unittest.cc  |   12 +++----
 5 files changed, 54 insertions(+), 33 deletions(-)

-----------------------------------------------------------------------
diff --git a/tests/tools/perfdhcp/command_options.cc b/tests/tools/perfdhcp/command_options.cc
index 8d9f02e..24e7e79 100644
--- a/tests/tools/perfdhcp/command_options.cc
+++ b/tests/tools/perfdhcp/command_options.cc
@@ -12,6 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <config.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -866,7 +867,7 @@ CommandOptions::usage() const {
 
 void
 CommandOptions::version() const {
-	fprintf(stdout, "version 0.01\n");
+    std::cout << "VERSION: " << VERSION << std::endl;
 }
 
 
diff --git a/tests/tools/perfdhcp/pkt_transform.h b/tests/tools/perfdhcp/pkt_transform.h
index 08563a2..1f57105 100644
--- a/tests/tools/perfdhcp/pkt_transform.h
+++ b/tests/tools/perfdhcp/pkt_transform.h
@@ -114,6 +114,8 @@ public:
     template<typename T>
     static void writeValueAt(dhcp::OptionBuffer& in_buffer, size_t dest_pos,
                         T val) {
+        // @todo consider replacing the loop with switch statement
+        // checking sizeof(T).
         for (int i = 0; i < sizeof(T); ++i) {
             in_buffer[dest_pos + i] = (val >> 8 * (sizeof(T) - i - 1)) & 0xFF;
         }
diff --git a/tests/tools/perfdhcp/test_control.cc b/tests/tools/perfdhcp/test_control.cc
index ef970a7..090fb32 100644
--- a/tests/tools/perfdhcp/test_control.cc
+++ b/tests/tools/perfdhcp/test_control.cc
@@ -299,8 +299,8 @@ TestControl::factoryOptionRequestOption6(Option::Universe,
                                          uint16_t,
                                          const OptionBuffer&) {
     const uint8_t buf_array[] = {
-        D6O_NAME_SERVERS, 0,
-        D6O_DOMAIN_SEARCH, 0,
+        0, D6O_NAME_SERVERS,
+        0, D6O_DOMAIN_SEARCH,
     };
     OptionBuffer buf_with_options(buf_array, buf_array + sizeof(buf_array));
     return (OptionPtr(new Option(Option::V6, D6O_ORO, buf_with_options)));
@@ -731,7 +731,7 @@ TestControl::readPacketTemplate(const std::string& file_name) {
 }
 
 void
-TestControl::receivePacket4(const TestControlSocket& socket,
+TestControl::processReceivedPacket4(const TestControlSocket& socket,
                             const Pkt4Ptr& pkt4) {
     if (pkt4->getType() == DHCPOFFER) {
         Pkt4Ptr discover_pkt4(stats_mgr4_->passRcvdPacket(StatsMgr4::XCHG_DO,
@@ -742,6 +742,8 @@ TestControl::receivePacket4(const TestControlSocket& socket,
             if (template_buffers_.size() < 2) {
                 sendRequest4(socket, discover_pkt4, pkt4);
             } else {
+                // @todo add defines for packet type index that can be
+                // used to access template_buffers_.
                 sendRequest4(socket, template_buffers_[1], discover_pkt4, pkt4);
             }
         }
@@ -751,7 +753,7 @@ TestControl::receivePacket4(const TestControlSocket& socket,
 }
 
 void
-TestControl::receivePacket6(const TestControlSocket& socket,
+TestControl::processReceivedPacket6(const TestControlSocket& socket,
                             const Pkt6Ptr& pkt6) {
     uint8_t packet_type = pkt6->getType();
     if (packet_type == DHCPV6_ADVERTISE) {
@@ -766,6 +768,8 @@ TestControl::receivePacket6(const TestControlSocket& socket,
             if (template_buffers_.size() < 2) {
                 sendRequest6(socket, pkt6);
             } else {
+                // @todo add defines for packet type index that can be
+                // used to access template_buffers_.
                 sendRequest6(socket, template_buffers_[1], pkt6);
             }
         }
@@ -790,7 +794,7 @@ TestControl::receivePackets(const TestControlSocket& socket) {
                     stats_mgr4_->incrementCounter("multircvd");
                 }
                 pkt4->unpack();
-                receivePacket4(socket, pkt4);
+                processReceivedPacket4(socket, pkt4);
             }
         } else if (CommandOptions::instance().getIpVersion() == 6) {
             Pkt6Ptr pkt6 = IfaceMgr::instance().receive6(timeout);
@@ -802,7 +806,7 @@ TestControl::receivePackets(const TestControlSocket& socket) {
                     stats_mgr6_->incrementCounter("multircvd");
                 }
                 if (pkt6->unpack()) {
-                    receivePacket6(socket, pkt6);
+                    processReceivedPacket6(socket, pkt6);
                 }
             }
         }
@@ -949,8 +953,9 @@ TestControl::run() {
             } else {
                 // Pick template #0 if Discover is being sent.
                 // For Request it would be #1.
-                const uint8_t template_idx = 0;
-                sendDiscover4(socket, template_buffers_[template_idx],
+                // @todo add defines for packet type index that can be
+                // used to access template_buffers_.
+                sendDiscover4(socket, template_buffers_[0],
                               do_preload);
             }
         } else if (options.getIpVersion() == 6) {
@@ -961,8 +966,9 @@ TestControl::run() {
             } else {
                 // Pick template #0 if Solicit is being sent.
                 // For Request it would be #1.
-                const uint8_t template_idx = 0;
-                sendSolicit6(socket, template_buffers_[template_idx],
+                // @todo add defines for packet type index that can be
+                // used to access template_buffers_.
+                sendSolicit6(socket, template_buffers_[0],
                              do_preload);
             }
         }
@@ -1000,8 +1006,9 @@ TestControl::run() {
                 if (template_buffers_.size() == 0) {
                     sendDiscover4(socket);
                 } else {
-                    const uint8_t template_idx = 0;
-                    sendDiscover4(socket, template_buffers_[template_idx]);
+                    // @todo add defines for packet type index that can be
+                    // used to access template_buffers_.
+                    sendDiscover4(socket, template_buffers_[0]);
                 }
             } else {
                 // No template packets means that no -T option was specified.
@@ -1009,8 +1016,9 @@ TestControl::run() {
                 if (template_buffers_.size() == 0) {
                     sendSolicit6(socket);
                 } else {
-                    const uint8_t template_idx = 0;
-                    sendSolicit6(socket, template_buffers_[template_idx]);
+                    // @todo add defines for packet type index that can be
+                    // used to access template_buffers_.
+                    sendSolicit6(socket, template_buffers_[0]);
                 }
             }
         }
diff --git a/tests/tools/perfdhcp/test_control.h b/tests/tools/perfdhcp/test_control.h
index 8e492af..1b0b83c 100644
--- a/tests/tools/perfdhcp/test_control.h
+++ b/tests/tools/perfdhcp/test_control.h
@@ -46,6 +46,12 @@ namespace perfdhcp {
 /// \ref dhcp::Option::factory with DHCP message type specified as one of
 ///  parameters. Some of the parameters passed to factory function
 /// may be ignored (e.g. option buffer).
+/// Please note that naming convention for factory functions within this
+/// class is as follows:
+/// - factoryABC4 - factory function for DHCPv4 option,
+/// - factoryDEF6 - factory function for DHCPv6 option,
+/// - factoryGHI - factory function that can be used to create either
+/// DHCPv4 or DHCPv6 option.
 class TestControl : public boost::noncopyable {
 public:
 
@@ -449,11 +455,11 @@ protected:
     /// not initialized.
     void printStats() const;
 
-    /// \brief Receive DHCPv4 packet.
+    /// \brief Process received DHCPv4 packet.
     ///
-    /// Method performs reception of the DHCPv4 packet, updates
-    /// statistics and responsds to the server if required, e.g.
-    /// when OFFER packet arrives, this function will initiate
+    /// Method performs processing of the received DHCPv4 packet,
+    /// updates statistics and responds to the server if required,
+    /// e.g. when OFFER packet arrives, this function will initiate
     /// REQUEST message to the server.
     ///
     /// \warning this method does not check if provided socket is
@@ -463,14 +469,14 @@ protected:
     /// \param [in] pkt4 object representing DHCPv4 packet received.
     /// \throw isc::BadValue if unknown message type received.
     /// \throw isc::Unexpected if unexpected error occured.
-    void receivePacket4(const TestControlSocket& socket,
-                        const dhcp::Pkt4Ptr& pkt4);
+    void processReceivedPacket4(const TestControlSocket& socket,
+                                const dhcp::Pkt4Ptr& pkt4);
 
-    /// \brief Receive DHCPv6 packet.
+    /// \brief Process received DHCPv6 packet.
     ///
-    /// Method performs reception of the DHCPv6 packet, updates
-    /// statistics and responsds to the server if required, e.g.
-    /// when ADVERTISE packet arrives, this function will initiate
+    /// Method performs processing of the received DHCPv6 packet,
+    /// updates statistics and responsds to the server if required,
+    /// e.g. when ADVERTISE packet arrives, this function will initiate
     /// REQUEST message to the server.
     ///
     /// \warning this method does not check if provided socket is
@@ -480,8 +486,8 @@ protected:
     /// \param [in] pkt6 object representing DHCPv6 packet received.
     /// \throw isc::BadValue if unknown message type received.
     /// \throw isc::Unexpected if unexpected error occured.
-    void receivePacket6(const TestControlSocket& socket,
-                        const dhcp::Pkt6Ptr& pkt6);
+    void processReceivedPacket6(const TestControlSocket& socket,
+                                const dhcp::Pkt6Ptr& pkt6);
 
     /// \brief Receive DHCPv4 or DHCPv6 packets from the server.
     ///
@@ -706,6 +712,8 @@ private:
 
     /// \brief Convert binary value to hex string.
     ///
+    /// \todo Consider moving this function to src/lib/util.
+    ///
     /// \param b byte to convert.
     /// \return hex string.
     std::string byte2Hex(const uint8_t b) const;
@@ -760,6 +768,8 @@ private:
 
     /// \brief Convert vector in hexadecimal string.
     ///
+    /// \todo Consider moving this function to src/lib/util.
+    ///
     /// \param vec vector to be converted.
     /// \param separator separator.
     std::string vector2Hex(const std::vector<uint8_t>& vec,
diff --git a/tests/tools/perfdhcp/tests/test_control_unittest.cc b/tests/tools/perfdhcp/tests/test_control_unittest.cc
index e5dbaf1..a4cde00 100644
--- a/tests/tools/perfdhcp/tests/test_control_unittest.cc
+++ b/tests/tools/perfdhcp/tests/test_control_unittest.cc
@@ -81,8 +81,8 @@ public:
     using TestControl::initPacketTemplates;
     using TestControl::initializeStatsMgr;
     using TestControl::openSocket;
-    using TestControl::receivePacket4;
-    using TestControl::receivePacket6;
+    using TestControl::processReceivedPacket4;
+    using TestControl::processReceivedPacket6;
     using TestControl::registerOptionFactories;
     using TestControl::sendDiscover4;
     using TestControl::sendSolicit6;
@@ -418,7 +418,7 @@ public:
             // packet drops.
             if (i < receive_num) {
                 boost::shared_ptr<Pkt4> offer_pkt4(createOfferPkt4(transid));
-                ASSERT_NO_THROW(tc.receivePacket4(sock, offer_pkt4));
+                ASSERT_NO_THROW(tc.processReceivedPacket4(sock, offer_pkt4));
                 ++transid;
             }
             if (tc.checkExitConditions()) {
@@ -483,7 +483,7 @@ public:
                 boost::shared_ptr<Pkt6>
                     advertise_pkt6(createAdvertisePkt6(transid));
                 // Receive ADVERTISE and send REQUEST.
-                ASSERT_NO_THROW(tc.receivePacket6(sock, advertise_pkt6));
+                ASSERT_NO_THROW(tc.processReceivedPacket6(sock, advertise_pkt6));
                 ++transid;
             }
             if (tc.checkExitConditions()) {
@@ -770,8 +770,8 @@ TEST_F(TestControlTest, Options6) {
     OptionPtr opt_oro(Option::factory(Option::V6, D6O_ORO));
     // Prepare the reference buffer with requested options.
     const uint8_t requested_options[] = {
-        D6O_NAME_SERVERS, 0,
-        D6O_DOMAIN_SEARCH, 0,
+        0, D6O_NAME_SERVERS,
+        0, D6O_DOMAIN_SEARCH,
     };
     int requested_options_num =
         sizeof(requested_options) / sizeof(requested_options[0]);



More information about the bind10-changes mailing list