[3806] Addressed some of the review comments.

Except user guide related and "fixme" leftovers.

diff --git a/src/bin/dhcp4/dhcp4_log.h b/src/bin/dhcp4/dhcp4_log.h
index 11af647c678111b03cfd116630941b2e43955d3d..2f1050cc140fc0df77dae8f5feac510000d061af 100644 (file)
--- a/src/bin/dhcp4/dhcp4_log.h
+++ b/src/bin/dhcp4/dhcp4_log.h
@@ -46,7 +46,7 @@ const int DBG_DHCP4_HOOKS = DBGLVL_TRACE_BASIC;
 /// @brief Debug level used to log the traces with some basic data.
 ///
 /// The basic data includes summary information, e.g. summary of the
-/// information returned by the particular function. It may also include
+/// information returned by a particular function. It may also include
 /// more detailed information in cases when it is warranted and the
 /// extraction of the data doesn't impact the server's performance
 /// significantly.

diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index fbdc53c0f773bcb0f1b59059d7b2dbbf57e58374..4dd7b430efda762c22998e56ef601296cf17c536 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -25,7 +25,7 @@ occurred during this attempt. The reason for the error is included in
 the message.
 
 % DHCP4_BUFFER_RECEIVED received buffer from %1:%2 to %3:%4 over interface %5
-This debug message is logged when the server has received some packet
+This debug message is logged when the server has received a packet
 over the socket. When the message is logged the contents of the received
 packet hasn't been parsed yet. The only available information is the
 interface and the source and destination addresses/ports.
@@ -72,11 +72,11 @@ information. The second argument includes all classes to which the
 packet has been assigned.
 
 % DHCP4_CLIENT_FQDN_PROCESS %1: processing Client FQDN option
-This debug message is issued when the server starts to process the Client
+This debug message is issued when the server starts processing the Client
 FQDN option sent in the client's query. The argument includes the
 client and transaction identification information.
 
-% DHCP4_CLIENT_FQDN_DATA %1: Client FQDN option information: %2
+% DHCP4_CLIENT_FQDN_DATA %1: Client sent FQDN option: %2
 This debug message includes the detailed information extracted from the
 Client FQDN option sent in the query. The first argument includes the
 client and transaction identification information. The second argument
@@ -84,11 +84,11 @@ specifies the detailed information about the FQDN option received
 by the server.
 
 % DHCP4_CLIENT_HOSTNAME_PROCESS %1: processing client's Hostname option
-This debug message is issued when the server starts to process the Hostname
+This debug message is issued when the server starts processing the Hostname
 option sent in the client's query. The argument includes the client and
 transaction identification information.
 
-% DHCP4_CLIENT_HOSTNAME_DATA %1: client Hostname option information: %2
+% DHCP4_CLIENT_HOSTNAME_DATA %1: client sent Hostname option: %2
 This debug message includes the detailed information extracted from the
 Hostname option sent in the query. The first argument includes the
 client and transaction identification information. The second argument
@@ -226,11 +226,12 @@ client class has reported a failure. The response packet will not be sent.
 The argument specifies the client and the transaction identification
 information.
 
-% DHCP4_INFORM_DIRECT_REPLY %1: DHCPACK in reply to the DHCPINFORM will be sent directly to %2
+% DHCP4_INFORM_DIRECT_REPLY %1: DHCPACK in reply to the DHCPINFORM will be sent directly to %2 over %3
 This debug message is issued when the DHCPACK will be sent directly to the
 client, rather than via a relay. The first argument contains the client
 and transaction identification information. The second argument contains
-the client's address to which the response will be sent.
+the client's address to which the response will be sent. The third argument
+contains the local interface name.
 
 % DHCP4_INIT_FAIL failed to initialize Kea server: %1
 The server has failed to initialize. This may be because the configuration
@@ -541,7 +542,7 @@ client class has reported a failure. The response packet will not be sent.
 The argument contains the client and transaction identification
 information.
 
-% DHCP4_RESPONSE_DATA %1: responding with packet %2 (type %3), packet details: "%4"
+% DHCP4_RESPONSE_DATA %1: responding with packet %2 (type %3), packet details: %4
 A debug message including the detailed data about the packet being sent
 to the client. The first argument contains the client and the transaction
 identification information. The second and third argument contains the

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 44622161b64b0acd0b74efe3bd6ef21368affec4..2a53b475dd2aca5f16ebeea4b3a4b0b17487198c 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -194,7 +194,6 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const bool use_bcast,
       use_bcast_(use_bcast), hook_index_pkt4_receive_(-1),
       hook_index_subnet4_select_(-1), hook_index_pkt4_send_(-1) {
 
-    // fixme: either remove or change its name.
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port);
     try {
         // Port 0 is used for testing purposes where we don't open broadcast
@@ -1791,7 +1790,8 @@ Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
     if (ack->getRemoteAddr() != inform->getGiaddr()) {
         LOG_DEBUG(packet_logger, DBG_DHCP4_DETAIL, DHCP4_INFORM_DIRECT_REPLY)
             .arg(inform->getLabel())
-            .arg(ack->getRemoteAddr());
+            .arg(ack->getRemoteAddr())
+            .arg(ack->getIface());
         ack->setHops(0);
         ack->setGiaddr(IOAddress::IPV4_ZERO_ADDRESS());
         ack->delOption(DHO_DHCP_AGENT_OPTIONS);

diff --git a/src/lib/dhcp/option_int_array.h b/src/lib/dhcp/option_int_array.h
index 450e253082fff6e72505d06f0644b58d48eb902a..40ca97a3274360e4bf6586334a9260d2d09b6ed8 100644 (file)
--- a/src/lib/dhcp/option_int_array.h
+++ b/src/lib/dhcp/option_int_array.h
@@ -262,7 +262,7 @@ public:
         std::string data_type = OptionDataTypeUtil::getDataTypeName(OptionDataTypeTraits<T>::type);
         for (typename std::vector<T>::const_iterator value = values_.begin();
              value != values_.end(); ++value) {
-            output << " " << *value << " (" << data_type << ")";
+            output << " " << *value << "(" << data_type << ")";
         }
 
         return (output.str());

diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc
index 3f5513f9e695da3467fc5d4ff120b9adc0e79e74..5435a2dad8ae65ed0b14f82f3bd7aabd284dd39f 100644 (file)
--- a/src/lib/dhcp/pkt4.cc
+++ b/src/lib/dhcp/pkt4.cc
@@ -362,8 +362,21 @@ Pkt4::toText() const {
     stringstream output;
     output << "local_address=" << local_addr_ << ":" << local_port_
         << ", remote_adress=" << remote_addr_
-        << ":" << remote_port_ << ", msg_type=" << static_cast<int>(getType())
-        << ", transid=0x" << hex << transid_ << dec;
+        << ":" << remote_port_ << ", msg_type=";
+
+    // Try to obtain message type. This may throw if the Message Type option is
+    // not present. Therefore we guard it with try-catch, because we don't want
+    // toText method to throw.
+    try {
+        uint8_t msg_type = getType();
+        output << getName(msg_type) << " (" << static_cast<int>(msg_type) << ")";
+
+    } catch (...) {
+        // Message Type option is missing.
+        output << "(missing)";
+    }
+
+    output << ", transid=0x" << hex << transid_ << dec;
 
     if (!options_.empty()) {
         output << "," << std::endl << "options:";
@@ -371,6 +384,9 @@ Pkt4::toText() const {
              opt != options_.end(); ++opt) {
             output << std::endl << opt->second->toText(2);
         }
+
+    } else {
+        output << ", message contains no options";
     }
 
     return (output.str());

diff --git a/src/lib/dhcp/tests/option_int_array_unittest.cc b/src/lib/dhcp/tests/option_int_array_unittest.cc
index 2a16feee4884317ff0e5286089448df75c02fb1a..6b94ed2a3282a7ec3ced7e20b7f6da5dc707e4ff 100644 (file)
--- a/src/lib/dhcp/tests/option_int_array_unittest.cc
+++ b/src/lib/dhcp/tests/option_int_array_unittest.cc
@@ -464,13 +464,15 @@ TEST_F(OptionIntArrayTest, addValuesInt32) {
     addValuesTest<int16_t>();
 }
 
+// This test checks that the option is correctly converted into
+// the textual format.
 TEST_F(OptionIntArrayTest, toText) {
     OptionUint32Array option(Option::V4, 128);
     option.addValue(1);
     option.addValue(32);
     option.addValue(324);
 
-    EXPECT_EQ("type=128, len=012: 1 (uint32) 32 (uint32) 324 (uint32)",
+    EXPECT_EQ("type=128, len=012: 1(uint32) 32(uint32) 324(uint32)",
               option.toText());
 }
 

diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc
index 03f75c8b87f221168f1ee9e16246651ce4b2e5d1..b954d6bc4bf91a4dcfbd4ec3bdeba16fb2a5b246 100644 (file)
--- a/src/lib/dhcp/tests/pkt4_unittest.cc
+++ b/src/lib/dhcp/tests/pkt4_unittest.cc
@@ -976,13 +976,26 @@ TEST_F(Pkt4Test, toText) {
     pkt.addOption(OptionPtr(new OptionString(Option::V4, 87, "lorem ipsum")));
 
     EXPECT_EQ("local_address=192.0.2.34:67, remote_adress=192.10.33.4:68, "
-              "msg_type=1, transid=0x9ef,\n"
+              "msg_type=DHCPDISCOVER (1), transid=0x9ef,\n"
               "options:\n"
               "  type=053, len=001: 01\n"
               "  type=087, len=011: \"lorem ipsum\" (string)\n"
               "  type=123, len=004: 192.0.2.3\n"
               "  type=156, len=004: 123456 (uint32)",
               pkt.toText());
+
+    // Now remove all options, including Message Type and check if the
+    // information about lack of any options is displayed properly.
+    pkt.delOption(123);
+    pkt.delOption(156);
+    pkt.delOption(87);
+    pkt.delOption(53);
+
+    EXPECT_EQ("local_address=192.0.2.34:67, remote_adress=192.10.33.4:68, "
+              "msg_type=(missing), transid=0x9ef, "
+              "message contains no options",
+              pkt.toText());
+
 }
 
 } // end of anonymous namespace