[2264] DHCP packet pack methods now throw exceptions on error.

Changed pack() methods for both DHCP4 and DHCP6 packets to
throw exceptions on failure, rather than return bools.

src/bin/dhcp4/dhcp4_srv.cc

@@ -320,14 +320,12 @@ Dhcpv4Srv::run() {
                           DHCP4_RESPONSE_DATA)
                           .arg(rsp->getType()).arg(rsp->toText());
 
-                if (rsp->pack()) {
-                    try {
-                        sendPacket(rsp);
-                    } catch (const std::exception& e) {
-                        LOG_ERROR(dhcp4_logger, DHCP4_PACKET_SEND_FAIL).arg(e.what());
-                    }
-                } else {
-                    LOG_ERROR(dhcp4_logger, DHCP4_PACK_FAIL);
+                try {
+                    rsp->pack();
+                    sendPacket(rsp);
+                } catch (const std::exception& e) {
+                    LOG_ERROR(dhcp4_logger, DHCP4_PACKET_SEND_FAIL)
+                              .arg(e.what());
                 }
             }
         }

src/bin/dhcp6/dhcp6_srv.cc

@@ -324,14 +324,12 @@ bool Dhcpv6Srv::run() {
                           DHCP6_RESPONSE_DATA)
                     .arg(static_cast<int>(rsp->getType())).arg(rsp->toText());
 
-                if (rsp->pack()) {
-                    try {
-                        sendPacket(rsp);
-                    } catch (const std::exception& e) {
-                        LOG_ERROR(dhcp6_logger, DHCP6_PACKET_SEND_FAIL).arg(e.what());
-                    }
-                } else {
-                    LOG_ERROR(dhcp6_logger, DHCP6_PACK_FAIL);
+                try {
+                    rsp->pack();
+                    sendPacket(rsp);
+                } catch (const std::exception& e) {
+                    LOG_ERROR(dhcp6_logger, DHCP6_PACKET_SEND_FAIL)
+                              .arg(e.what());
                 }
             }
         }

src/lib/dhcp/pkt4.cc

@@ -102,55 +102,60 @@ Pkt4::len() {
     return (length);
 }
 
-bool
+void
 Pkt4::pack() {
     if (!hwaddr_) {
         isc_throw(InvalidOperation, "Can't build Pkt4 packet. HWAddr not set.");
     }
 
-    size_t hw_len = hwaddr_->hwaddr_.size();
-
-    bufferOut_.writeUint8(op_);
-    bufferOut_.writeUint8(hwaddr_->htype_);
-    bufferOut_.writeUint8(hw_len < MAX_CHADDR_LEN ? hw_len : MAX_CHADDR_LEN);
-    bufferOut_.writeUint8(hops_);
-    bufferOut_.writeUint32(transid_);
-    bufferOut_.writeUint16(secs_);
-    bufferOut_.writeUint16(flags_);
-    bufferOut_.writeUint32(ciaddr_);
-    bufferOut_.writeUint32(yiaddr_);
-    bufferOut_.writeUint32(siaddr_);
-    bufferOut_.writeUint32(giaddr_);
-
-
-    if (hw_len <= MAX_CHADDR_LEN) {
-        // write up to 16 bytes of the hardware address (CHADDR field is 16
-        // bytes long in DHCPv4 message).
-        bufferOut_.writeData(&hwaddr_->hwaddr_[0],
-                             (hw_len < MAX_CHADDR_LEN ? hw_len : MAX_CHADDR_LEN) );
-        hw_len = MAX_CHADDR_LEN - hw_len;
-    } else {
-        hw_len = MAX_CHADDR_LEN;
+    try {
+        size_t hw_len = hwaddr_->hwaddr_.size();
+
+        bufferOut_.writeUint8(op_);
+        bufferOut_.writeUint8(hwaddr_->htype_);
+        bufferOut_.writeUint8(hw_len < MAX_CHADDR_LEN ? 
+                              hw_len : MAX_CHADDR_LEN);
+        bufferOut_.writeUint8(hops_);
+        bufferOut_.writeUint32(transid_);
+        bufferOut_.writeUint16(secs_);
+        bufferOut_.writeUint16(flags_);
+        bufferOut_.writeUint32(ciaddr_);
+        bufferOut_.writeUint32(yiaddr_);
+        bufferOut_.writeUint32(siaddr_);
+        bufferOut_.writeUint32(giaddr_);
+
+
+        if (hw_len <= MAX_CHADDR_LEN) {
+            // write up to 16 bytes of the hardware address (CHADDR field is 16
+            // bytes long in DHCPv4 message).
+            bufferOut_.writeData(&hwaddr_->hwaddr_[0], 
+                                 (hw_len < MAX_CHADDR_LEN ? 
+                                  hw_len : MAX_CHADDR_LEN) );
+            hw_len = MAX_CHADDR_LEN - hw_len;
+        } else {
+            hw_len = MAX_CHADDR_LEN;
+        }
+
+        // write (len) bytes of padding
+        vector<uint8_t> zeros(hw_len, 0);
+        bufferOut_.writeData(&zeros[0], hw_len);
+        // bufferOut_.writeData(chaddr_, MAX_CHADDR_LEN);
+
+        bufferOut_.writeData(sname_, MAX_SNAME_LEN);
+        bufferOut_.writeData(file_, MAX_FILE_LEN);
+
+        // write DHCP magic cookie
+        bufferOut_.writeUint32(DHCP_OPTIONS_COOKIE);
+
+        LibDHCP::packOptions(bufferOut_, options_);
+
+        // add END option that indicates end of options
+        // (End option is very simple, just a 255 octet)
+         bufferOut_.writeUint8(DHO_END);
+     } catch(const Exception& e) {
+        // An exception is thrown and message will be written to Logger
+         isc_throw(InvalidOperation, e.what());
     }
-
-    // write (len) bytes of padding
-    vector<uint8_t> zeros(hw_len, 0);
-    bufferOut_.writeData(&zeros[0], hw_len);
-    // bufferOut_.writeData(chaddr_, MAX_CHADDR_LEN);
-
-    bufferOut_.writeData(sname_, MAX_SNAME_LEN);
-    bufferOut_.writeData(file_, MAX_FILE_LEN);
-
-    // write DHCP magic cookie
-    bufferOut_.writeUint32(DHCP_OPTIONS_COOKIE);
-
-    LibDHCP::packOptions(bufferOut_, options_);
-
-    // add END option that indicates end of options
-    // (End option is very simple, just a 255 octet)
-    bufferOut_.writeUint8(DHO_END);
-
-    return (true);
 }
 
 void

src/lib/dhcp/pkt4.h

@@ -72,8 +72,8 @@ public:
     /// Options must be stored in options_ field.
     /// Output buffer will be stored in bufferOut_.
     ///
-    /// @return true if packing procedure was successful
-    bool
+    /// @throw InvalidOperation if packing fails
+    void
     pack();
 
     /// @brief Parses on-wire form of DHCPv4 packet.

src/lib/dhcp/pkt6.cc

@@ -181,20 +181,21 @@ uint16_t Pkt6::directLen() const {
 }
 
 
-bool
+void
 Pkt6::pack() {
     switch (proto_) {
     case UDP:
-        return packUDP();
+        packUDP();
+        break;
     case TCP:
-        return packTCP();
+        packTCP();
+        break;
     default:
         isc_throw(BadValue, "Invalid protocol specified (non-TCP, non-UDP)");
     }
-    return (false); // never happens
 }
 
-bool
+void
 Pkt6::packUDP() {
     try {
 
@@ -252,13 +253,12 @@ Pkt6::packUDP() {
         LibDHCP::packOptions(bufferOut_, options_);
     }
     catch (const Exception& e) {
-        /// @todo: throw exception here once we turn this function to void.
-        return (false);
+       // An exception is thrown and message will be written to Logger
+       isc_throw(InvalidOperation, e.what());
     }
-    return (true);
 }
 
-bool
+void
 Pkt6::packTCP() {
     /// TODO Implement this function.
     isc_throw(Unexpected, "DHCPv6 over TCP (bulk leasequery and failover)"

src/lib/dhcp/pkt6.h

@@ -116,8 +116,10 @@ public:
     /// Output buffer will be stored in data_. Length
     /// will be set in data_len_.
     ///
-    /// @return true if packing procedure was successful
-    bool pack();
+    /// @throw BadValue if packet protocol is invalid, InvalidOperation
+    /// if packing fails, or Unexpected if protocol is TCP (IPv6 over TCP is 
+    /// not yet supported).
+    void pack();
 
     /// @brief Dispatch method that handles binary packet parsing.
     ///
@@ -415,13 +417,13 @@ protected:
     ///
     /// TODO This function is not implemented yet.
     ///
-    /// @return true, if build was successful
-    bool packTCP();
+    /// Method with throw exception if build fails
+    void packTCP();
 
     /// Builds on wire packet for UDP transmission.
     ///
-    /// @return true, if build was successful
-    bool packUDP();
+    /// Method with throw exception if build fails
+    void packUDP();
 
     /// @brief Parses on-wire form of TCP DHCPv6 packet.
     ///

src/lib/dhcp/tests/pkt6_unittest.cc

@@ -221,7 +221,7 @@ TEST_F(Pkt6Test, packUnpack) {
     EXPECT_EQ(Pkt6::DHCPV6_PKT_HDR_LEN + 3 * Option::OPTION6_HDR_LEN,
               parent->len());
 
-    EXPECT_TRUE(parent->pack());
+    EXPECT_NO_THROW(parent->pack());
 
     EXPECT_EQ(Pkt6::DHCPV6_PKT_HDR_LEN + 3 * Option::OPTION6_HDR_LEN,
               parent->len());
@@ -515,7 +515,7 @@ TEST_F(Pkt6Test, relayPack) {
 
     EXPECT_EQ(DHCPV6_ADVERTISE, parent->getType());
 
-    EXPECT_TRUE(parent->pack());
+    EXPECT_NO_THROW(parent->pack());
 
     EXPECT_EQ(Pkt6::DHCPV6_PKT_HDR_LEN + 3 * Option::OPTION6_HDR_LEN // ADVERTISE
               + Pkt6::DHCPV6_RELAY_HDR_LEN // Relay header