[3743] More review clean up, use ClientId for client info output

src/bin/dhcp4/dhcp4_srv.cc

@@ -404,7 +404,8 @@ Dhcpv4Srv::run() {
                 // Failed to parse the packet.
                 LOG_DEBUG(bad_packet_logger, DBG_DHCP4_DETAIL,
                           DHCP4_PACKET_DROP_0001)
-                    .arg(query->getLabel()).arg(e.what());
+                    .arg(query->getLabel())
+                    .arg(e.what());
                 continue;
             }
         }

src/lib/dhcp/pkt.h

@@ -148,7 +148,7 @@ public:
     /// there own implementation.
     ///
     /// @return string with text representation
-    virtual std::string getLabel() {
+    virtual std::string getLabel() const {
         isc_throw(NotImplemented, "Pkt::getLabel()");
     }
 

src/lib/dhcp/pkt4.cc

@@ -282,20 +282,29 @@ void Pkt4::setType(uint8_t dhcp_type) {
 }
 
 std::string
-Pkt4::getLabel() {
-    return makeLabel(hwaddr_, getOption(DHO_DHCP_CLIENT_IDENTIFIER), transid_);
-}
+Pkt4::getLabel() const {
+
+    /// @TODO If and when client id is extracted into Pkt4, this method should
+    /// the instance member rather than fetch it every time.
+    ClientIdPtr client_id;
+    OptionPtr client_opt = getOption(DHO_DHCP_CLIENT_IDENTIFIER);
+    if (client_opt ) {
+        client_id = ClientIdPtr(new ClientId(client_opt->getData()));
+    }
 
-std::string
-Pkt4::makeLabel(HWAddrPtr hwaddr, OptionPtr client_id, uint32_t transid)
-{
-    stringstream tmp;
+    return makeLabel(hwaddr_, client_id, transid_);
 
-    tmp << "hwaddr=[" << (hwaddr ? hwaddr->toText() : "no info")
-        << "], client-id=[" << (client_id ? client_id->toText() : "no info")
-        << "], transid=0x" << hex << transid << dec;
+}
 
-    return tmp.str();
+std::string
+Pkt4::makeLabel(const HWAddrPtr hwaddr, const ClientIdPtr client_id,
+                const uint32_t transid) {
+    stringstream label;
+    label << "hwaddr=[" << (hwaddr ? hwaddr->toText() : "no info")
+          << "], client-id=[" << (client_id ? client_id->toText() : "no info")
+          << "], transid=0x" << hex << transid << dec;
+
+    return label.str();
 }
 
 

src/lib/dhcp/pkt4.h

@@ -17,6 +17,7 @@
 
 #include <asiolink/io_address.h>
 #include <dhcp/option.h>
+#include <dhcp/duid.h>
 #include <util/buffer.h>
 #include <dhcp/option.h>
 #include <dhcp/classify.h>
@@ -108,26 +109,28 @@ public:
     /// Method will throw exception if anomaly is found.
     void check();
 
-    /// @brief Returns text representation primary packet identifiers
+    /// @brief Returns text representation of the primary packet identifiers
     ///
     /// This method is intended to be used to provide a consistent way to
     /// identify packets within log statements.  It is an instance-level
-    /// wrapper around static makeLabel()(). See this method for string
+    /// wrapper around static makeLabel(). See this method for string
     /// content.
     ///
     /// @return string with text representation
-    std::string getLabel();
+    std::string getLabel() const;
 
     /// @brief Returns text representation of the given packet identifiers
     ///
-    /// @param hwaddr - hardware address to include in the string
-    /// @param client_id - DHO_DHCP_CLIENT_ID_OPTION containing the client id
+    /// @param hwaddr - hardware address to include in the string, it may be
+    /// NULL.
+    /// @param client_id - client id to include in the string, it may be NULL.
     /// to include in the string
     /// @param transid - numeric transaction id to include in the string
     ///
     /// @return string with text representation
-    static std::string makeLabel(HWAddrPtr hwaddr, OptionPtr client_id,
-                                 uint32_t transid);
+    static std::string makeLabel(const HWAddrPtr hwaddr,
+                                 const ClientIdPtr client_id,
+                                 const uint32_t transid);
 
     /// @brief Returns text representation of the packet.
     ///

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -885,7 +885,7 @@ TEST_F(Pkt4Test, getLabel) {
 
     // Verify makeLabel() handles empty values
     EXPECT_EQ ("hwaddr=[no info], client-id=[no info], transid=0x0",
-               Pkt4::makeLabel(HWAddrPtr(), OptionPtr(), 0));
+               Pkt4::makeLabel(HWAddrPtr(), ClientIdPtr(), 0));
 
     // Verify an "empty" packet label is as we expect
     EXPECT_EQ ("hwaddr=[hwtype=1 ], client-id=[no info], transid=0x4d2",
@@ -911,7 +911,7 @@ TEST_F(Pkt4Test, getLabel) {
     pkt.addOption(opt);
 
     EXPECT_EQ ("hwaddr=[hwtype=123 02:04:06:08:0a:0c],"
-               " client-id=[type=61, len=4: 64:65:66:67], transid=0x4d2",
+               " client-id=[64:65:66:67], transid=0x4d2",
                pkt.getLabel());
 
 }