[1545] Simplify code for printing the type of packet received

src/bin/dhcp4/dhcp4_messages.mes

@@ -38,19 +38,14 @@ server but it is not running.
 A debug message issued during startup, this indicates that the IPv4 DHCP
 server is about to open sockets on the named port.
 
-% DHCP4_PACKET_DECLINE DECLINE packet received on interface %1
-The server has received a DECLINE packet on the specified interface.
-
-% DHCP4_PACKET_DISCOVER DISCOVER packet received on interface %1
-The server has received a DISCOVER packet on the specified interface.
-
-% DHCP4_PACKET_INFORM INFORM packet received on interface %1
-The server has received a DISCOVER packet on the specified interface.
-
 % DHCP4_PACKET_PARSE_FAIL failed to parse incoming packet: %1
 The server has received a packet that it is unable to interpret. The reason why
 the packet is invalid is include in the message.
 
+% DHCP4_PACKET_RECEIVED %1 (type %2) packet received on interface %3
+A debug message noting that the server has received the specified type of packet
+on the specified interface.
+
 % DHCP4_PACKET_RELEASE RELEASE packet received on interface %1
 The server has received a RELEASE packet on the specified interface.
 
@@ -67,10 +62,10 @@ but which should not be sent to the server (e.g. an OFFER or ACK).
 This error is output if the server failed to assemble the data to be returned to the client into
 a valid packet.  Additional messages will detail the reason.
 
-% DHCP4_QUERY_DATA responding with packet type %1 data is <<%2>>
-A debug message listing the data returned to the client.
+% DHCP4_QUERY_DATA received packet type %1, data is <<%2>>
+A debug message listing the data received from the client.
 
-% DHCP4_RESPONSE_DATA responding with packet type %1 data is <<%2>>
+% DHCP4_RESPONSE_DATA responding with packet type %1, data is <<%2>>
 A debug message listing the data returned to the client.
 
 % DHCP4_SERVER_FAILED server failed: %1

src/bin/dhcp4/dhcp4_srv.cc

@@ -39,12 +39,11 @@ const std::string HARDCODED_SERVER_ID = "192.0.2.1";
 Dhcpv4Srv::Dhcpv4Srv(uint16_t port) {
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port);
     try {
-        // first call to instance() will create IfaceMgr (it's a singleton)
+        // First call to instance() will create IfaceMgr (it's a singleton)
         // it may throw something if things go wrong
         IfaceMgr::instance();
 
         /// @todo: instantiate LeaseMgr here once it is imlpemented.
-
         IfaceMgr::instance().openSockets4(port);
 
         setServerID();
@@ -87,10 +86,13 @@ Dhcpv4Srv::run() {
                           DHCP4_PACKET_PARSE_FAIL).arg(e.what());
                 continue;
             }
+            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_UNKNOWN)
+                      .arg(serverReceivedPacketName(query->getType()))
+                      .arg(query->getType())
+                      .arg(query->getIface());
             LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_QUERY_DATA)
                       .arg(query->toText());
 
-
             switch (query->getType()) {
             case DHCPDISCOVER:
                 rsp = processDiscover(query);
@@ -113,8 +115,10 @@ Dhcpv4Srv::run() {
                 break;
 
             default:
-                LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_UNKNOWN)
-                          .arg(query->getType()).arg(query->getIface());
+                // Only action is to output a message if debug is enabled,
+                // and that will be covered by the debug statement before
+                // the "switch" statement.
+                ;
             }
 
             if (rsp) {
@@ -243,8 +247,6 @@ void Dhcpv4Srv::tryAssignLease(Pkt4Ptr& msg) {
 }
 
 Pkt4Ptr Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
-    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_DISCOVER)
-              .arg(discover->getIface());
     Pkt4Ptr offer = Pkt4Ptr
         (new Pkt4(DHCPOFFER, discover->getTransid()));
 
@@ -258,8 +260,6 @@ Pkt4Ptr Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
 }
 
 Pkt4Ptr Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
-    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_REQUEST)
-              .arg(request->getIface());
     Pkt4Ptr ack = Pkt4Ptr
         (new Pkt4(DHCPACK, request->getTransid()));
 
@@ -273,20 +273,45 @@ Pkt4Ptr Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
 }
 
 void Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
-    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_RELEASE)
-              .arg(release->getIface());
     /// TODO: Implement this.
 }
 
 void Dhcpv4Srv::processDecline(Pkt4Ptr& decline) {
-    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_DECLINE)
-              .arg(decline->getIface());
     /// TODO: Implement this.
 }
 
 Pkt4Ptr Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
-    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_INFORM)
-              .arg(inform->getIface());
     /// TODO: Currently implemented echo mode. Implement this for real
     return (inform);
 }
+
+const char*
+Dhcpv4Srv::serverReceivedPacketName(uint8_t type) {
+    static const char* DISCOVER = "DISCOVER";
+    static const char* REQUEST = "REQUEST";
+    static const char* RELEASE = "RELEASE";
+    static const char* DECLINE = "DECLINE";
+    static const char* INFORM = "INFORM";
+    static const char* UNKNOWN = "UNKNOWN";
+
+    switch (type) {
+    case DHCPDISCOVER:
+        return (DISCOVER);
+
+    case DHCPREQUEST:
+        return (REQUEST);
+
+    case DHCPRELEASE:
+        return (RELEASE);
+
+    case DHCPDECLINE:
+        return (DECLINE);
+
+    case DHCPINFORM:
+        return (INFORM);
+
+    default:
+        ;
+    }
+    return (UNKNOWN);
+}

src/bin/dhcp4/dhcp4_srv.h

@@ -70,6 +70,23 @@ class Dhcpv4Srv : public boost::noncopyable {
     /// @brief Instructs the server to shut down.
     void shutdown();
 
+    /// @brief Return textual type of packet received by server
+    ///
+    /// Returns the name of valid packet received by the server (e.g. DISCOVER).
+    /// If the packet is unknown - or if it is a valid DHCP packet but not one
+    /// expected to be received by the server (such as an OFFER), the string
+    /// "UNKNOWN" is returned.  This methos is used in debug messages.
+    ///
+    /// As the operation of the method does not depend on any server state, it
+    /// is declared static.
+    ///
+    /// @param type DHCPv4 packet type
+    ///
+    /// @return Pointer to "const" string containing the packet name.
+    ///         Note that this string is statically allocated and MUST NOT
+    ///         be freed by the caller.
+    static const char* serverReceivedPacketName(uint8_t type);
+
 protected:
     /// @brief Processes incoming DISCOVER and returns response.
     ///

src/bin/dhcp4/main.cc

@@ -14,13 +14,15 @@
 
 #include <config.h>
 #include <iostream>
+
+#include <boost/lexical_cast.hpp>
+
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <dhcp4/dhcp4_log.h>
 #include <log/logger_support.h>
-#include <boost/lexical_cast.hpp>
 
-using namespace std;
 using namespace isc::dhcp;
+using namespace std;
 
 /// This file contains entry point (main() function) for standard DHCPv4 server
 /// component for BIND10 framework. It parses command-line arguments and
@@ -37,11 +39,10 @@ const char* const DHCP4_NAME = "b10-dhcp4";
 
 void
 usage() {
-    cerr << "Usage:  b10-dhcp4 [-v]"
-         << endl;
-    cerr << "\t-v: verbose output" << endl;
-    cerr << "\t-s: stand-alone mode (don't connect to BIND10)" << endl;
-    cerr << "\t-p number: specify non-standard port number 1-65535 "
+    cerr << "Usage: " << DHCP4_NAME << " [-v] [-s] [-p number]" << endl;
+    cerr << "  -v: verbose output" << endl;
+    cerr << "  -s: stand-alone mode (don't connect to BIND10)" << endl;
+    cerr << "  -p number: specify non-standard port number 1-65535 "
          << "(useful for testing only)" << endl;
     exit(EXIT_FAILURE);
 }
@@ -50,10 +51,10 @@ usage() {
 int
 main(int argc, char* argv[]) {
     int ch;
-    bool verbose_mode = false; // should server be verbose?
     int port_number = DHCP4_SERVER_PORT; // The default. any other values are
                                          // useful for testing only.
-    bool stand_alone = false; // should be connect to BIND10 msgq?
+    bool stand_alone = false;  // Should be connect to BIND10 msgq?
+    bool verbose_mode = false; // Should server be verbose?
 
     while ((ch = getopt(argc, argv, "vsp:")) != -1) {
         switch (ch) {
@@ -86,7 +87,7 @@ main(int argc, char* argv[]) {
     }
 
     // Check for extraneous parameters.
-    if ((argc - optind) > 0) {
+    if (argc > optind) {
         usage();
     }
 
@@ -118,8 +119,8 @@ main(int argc, char* argv[]) {
         LOG_INFO(dhcp4_logger, DHCP4_SHUTDOWN);
 
     } catch (const std::exception& ex) {
-        ret = EXIT_FAILURE;
         LOG_FATAL(dhcp4_logger, DHCP4_SERVER_FAILED).arg(ex.what());
+        ret = EXIT_FAILURE;
     }
 
     return (ret);

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -271,4 +271,36 @@ TEST_F(Dhcpv4SrvTest, processInform) {
     delete srv;
 }
 
+TEST_F(Dhcpv4SrvTest, serverReceivedPacketName) {
+    // Check all possible packet types
+    for (int itype = 0; itype < 256; ++itype) {
+        uint8_t type = itype;
+
+        switch (type) {
+        case DHCPDECLINE:
+            EXPECT_STREQ("DECLINE", Dhcpv4Srv::serverReceivedPacketName(type));
+            break;
+
+        case DHCPDISCOVER:
+            EXPECT_STREQ("DISCOVER", Dhcpv4Srv::serverReceivedPacketName(type));
+            break;
+
+        case DHCPINFORM:
+            EXPECT_STREQ("INFORM", Dhcpv4Srv::serverReceivedPacketName(type));
+            break;
+
+        case DHCPRELEASE:
+            EXPECT_STREQ("RELEASE", Dhcpv4Srv::serverReceivedPacketName(type));
+            break;
+
+        case DHCPREQUEST:
+            EXPECT_STREQ("REQUEST", Dhcpv4Srv::serverReceivedPacketName(type));
+            break;
+
+        default:
+            EXPECT_STREQ("UNKNOWN", Dhcpv4Srv::serverReceivedPacketName(type));
+        }
+    }
+}
+
 } // end of anonymous namespace