[1959] Implemented suggestions from code review.

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;
 }
 
 

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;
         }

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]);
                 }
             }
         }

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,

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]);