[master] Merge branch 'trac3200'

src/bin/dhcp4/dhcp4_messages.mes

@@ -118,7 +118,7 @@ a lease. It is up to the client to choose one server out of othe advertised
 and continue allocation with that server. This is a normal behavior and
 indicates successful operation.
 
-% DHCP4_LEASE_ADVERT_FAIL failed to advertise a lease for client client-id %1, hwaddr %2
+% DHCP4_LEASE_ADVERT_FAIL failed to advertise a lease for client client-id %1, hwaddr %2, client sent yiaddr %3
 This message indicates that the server has failed to offer a lease to
 the specified client after receiving a DISCOVER message from it. There are
 many possible reasons for such a failure.
@@ -128,7 +128,7 @@ This debug message indicates that the server successfully granted a lease
 in response to client's REQUEST message. This is a normal behavior and
 indicates successful operation.
 
-% DHCP4_LEASE_ALLOC_FAIL failed to grant a lease for client-id %1, hwaddr %2
+% DHCP4_LEASE_ALLOC_FAIL failed to grant a lease for client-id %1, hwaddr %2, client sent yiaddr %3
 This message indicates that the server failed to grant a lease to the
 specified client after receiving a REQUEST message from it.  There are many
 possible reasons for such a failure. Additional messages will indicate the

src/bin/dhcp4/dhcp4_srv.cc

@@ -754,13 +754,6 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
         opt->setUint32(lease->valid_lft_);
         answer->addOption(opt);
 
-        // Router (type 3)
-        Subnet::OptionDescriptor opt_routers =
-            subnet->getOptionDescriptor("dhcp4", DHO_ROUTERS);
-        if (opt_routers.option) {
-            answer->addOption(opt_routers.option);
-        }
-
         // Subnet mask (type 1)
         answer->addOption(getNetmaskOption(subnet));
 
@@ -857,14 +850,17 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
 
     copyDefaultFields(discover, offer);
     appendDefaultOptions(offer, DHCPOFFER);
-    appendRequestedOptions(discover, offer);
 
     assignLease(discover, offer);
 
-    // There are a few basic options that we always want to
-    // include in the response. If client did not request
-    // them we append them for him.
-    appendBasicOptions(discover, offer);
+    // Adding any other options makes sense only when we got the lease.
+    if (offer->getYiaddr() != IOAddress("0.0.0.0")) {
+        appendRequestedOptions(discover, offer);
+        // There are a few basic options that we always want to
+        // include in the response. If client did not request
+        // them we append them for him.
+        appendBasicOptions(discover, offer);
+    }
 
     return (offer);
 }
@@ -880,17 +876,20 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
 
     copyDefaultFields(request, ack);
     appendDefaultOptions(ack, DHCPACK);
-    appendRequestedOptions(request, ack);
 
     // Note that we treat REQUEST message uniformly, regardless if this is a
     // first request (requesting for new address), renewing existing address
     // or even rebinding.
     assignLease(request, ack);
 
-    // There are a few basic options that we always want to
-    // include in the response. If client did not request
-    // them we append them for him.
-    appendBasicOptions(request, ack);
+    // Adding any other options makes sense only when we got the lease.
+    if (ack->getYiaddr() != IOAddress("0.0.0.0")) {
+        appendRequestedOptions(request, ack);
+        // There are a few basic options that we always want to
+        // include in the response. If client did not request
+        // them we append them for him.
+        appendBasicOptions(request, ack);
+    }
 
     return (ack);
 }

src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -22,6 +22,7 @@
 #include <dhcp/option_custom.h>
 #include <dhcp/iface_mgr.h>
 #include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/lease.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 
@@ -123,37 +124,101 @@ void Dhcpv4SrvTest::messageCheck(const Pkt4Ptr& q, const Pkt4Ptr& a) {
     EXPECT_TRUE(q->getLocalHWAddr() == a->getLocalHWAddr());
     EXPECT_TRUE(q->getRemoteHWAddr() == a->getRemoteHWAddr());
 
-    // Check that bare minimum of required options are there.
-    // We don't check options requested by a client. Those
-    // are checked elsewhere.
-    EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
-    EXPECT_TRUE(a->getOption(DHO_ROUTERS));
+    // Check that the server identifier is present in the response.
+    // Presence (or absence) of other options is checked elsewhere.
     EXPECT_TRUE(a->getOption(DHO_DHCP_SERVER_IDENTIFIER));
-    EXPECT_TRUE(a->getOption(DHO_DHCP_LEASE_TIME));
-    EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
-    EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME));
-    EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME_SERVERS));
 
     // Check that something is offered
     EXPECT_TRUE(a->getYiaddr().toText() != "0.0.0.0");
 }
 
-void Dhcpv4SrvTest::optionsCheck(const Pkt4Ptr& pkt) {
-    // Check that the requested and configured options are returned
-    // in the ACK message.
-    EXPECT_TRUE(pkt->getOption(DHO_DOMAIN_NAME))
-        << "domain-name not present in the response";
-    EXPECT_TRUE(pkt->getOption(DHO_DOMAIN_NAME_SERVERS))
-        << "dns-servers not present in the response";
-    EXPECT_TRUE(pkt->getOption(DHO_LOG_SERVERS))
-        << "log-servers not present in the response";
-    EXPECT_TRUE(pkt->getOption(DHO_COOKIE_SERVERS))
-        << "cookie-servers not present in the response";
-    // Check that the requested but not configured options are not
-    // returned in the ACK message.
-    EXPECT_FALSE(pkt->getOption(DHO_LPR_SERVERS))
-        << "domain-name present in the response but it is"
-        << " expected not to be present";
+::testing::AssertionResult
+Dhcpv4SrvTest::basicOptionsPresent(const Pkt4Ptr& pkt) {
+    std::ostringstream errmsg;
+    errmsg << "option missing in the response";
+    if (!pkt->getOption(DHO_DOMAIN_NAME)) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "domain-name " << errmsg));
+
+    } else if (!pkt->getOption(DHO_DOMAIN_NAME_SERVERS)) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "dns-servers " << errmsg));
+
+    } else if (!pkt->getOption(DHO_SUBNET_MASK)) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "subnet-mask " << errmsg));
+
+    } else if (!pkt->getOption(DHO_ROUTERS)) {
+        return (::testing::AssertionFailure(::testing::Message() << "routers "
+                                            << errmsg));
+
+    } else if (!pkt->getOption(DHO_DHCP_LEASE_TIME)) {
+        return (::testing::AssertionFailure(::testing::Message() <<
+                                            "dhcp-lease-time " << errmsg));
+
+    }
+    return (::testing::AssertionSuccess());
+
+}
+
+::testing::AssertionResult
+Dhcpv4SrvTest::noBasicOptions(const Pkt4Ptr& pkt) {
+    std::ostringstream errmsg;
+    errmsg << "option present in the response";
+    if (pkt->getOption(DHO_DOMAIN_NAME)) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "domain-name " << errmsg));
+
+    } else if (pkt->getOption(DHO_DOMAIN_NAME_SERVERS)) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "dns-servers " << errmsg));
+
+    } else if (pkt->getOption(DHO_SUBNET_MASK)) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "subnet-mask " << errmsg));
+
+    } else if (pkt->getOption(DHO_ROUTERS)) {
+        return (::testing::AssertionFailure(::testing::Message() << "routers "
+                                            << errmsg));
+
+    } else if (pkt->getOption(DHO_DHCP_LEASE_TIME)) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "dhcp-lease-time " << errmsg));
+
+    }
+    return (::testing::AssertionSuccess());
+}
+
+::testing::AssertionResult
+Dhcpv4SrvTest::requestedOptionsPresent(const Pkt4Ptr& pkt) {
+    std::ostringstream errmsg;
+    errmsg << "option missing in the response";
+    if (!pkt->getOption(DHO_LOG_SERVERS)) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "log-servers " << errmsg));
+
+    } else if (!pkt->getOption(DHO_COOKIE_SERVERS)) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "cookie-servers " << errmsg));
+
+    }
+    return (::testing::AssertionSuccess());
+}
+
+::testing::AssertionResult
+Dhcpv4SrvTest::noRequestedOptions(const Pkt4Ptr& pkt) {
+    std::ostringstream errmsg;
+    errmsg << "option present in the response";
+    if (pkt->getOption(DHO_LOG_SERVERS)) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "log-servers " << errmsg));
+
+    } else if (pkt->getOption(DHO_COOKIE_SERVERS)) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "cookie-servers " << errmsg));
+
+    }
+    return (::testing::AssertionSuccess());
 }
 
 OptionPtr Dhcpv4SrvTest::generateClientId(size_t size /*= 4*/) {
@@ -274,6 +339,48 @@ void Dhcpv4SrvTest::checkClientId(const Pkt4Ptr& rsp, const OptionPtr& expected_
     EXPECT_TRUE(expected_clientid->getData() == opt->getData());
 }
 
+::testing::AssertionResult
+Dhcpv4SrvTest::createPacketFromBuffer(const Pkt4Ptr& src_pkt,
+                                      Pkt4Ptr& dst_pkt) {
+    // Create on-wire format of the packet. If pack() has been called
+    // on this instance of the packet already, the next call to pack()
+    // should remove all contents of the output buffer.
+    try {
+        src_pkt->pack();
+    } catch (const Exception& ex) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "Failed to parse source packet: "
+                                            << ex.what()));
+    }
+    // Get the output buffer from the source packet.
+    const util::OutputBuffer& buf = src_pkt->getBuffer();
+    // Create a copy of the packet using the output buffer from the source
+    // packet.
+    try {
+        dst_pkt.reset(new Pkt4(static_cast<const uint8_t*>(buf.getData()),
+                               buf.getLength()));
+    } catch (const Exception& ex) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "Failed to create a"
+                                            " destination packet from"
+                                            " the buffer: "
+                                            << ex.what()));
+    }
+
+    try {
+        // Parse the new packet and return to the caller.
+        dst_pkt->unpack();
+    } catch (const Exception& ex) {
+        return (::testing::AssertionFailure(::testing::Message()
+                                            << "Failed to parse a"
+                                            << " destination packet: "
+                                            << ex.what()));
+    }
+
+    return (::testing::AssertionSuccess());
+}
+
+
 void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
     // Create an instance of the tested class.
     boost::scoped_ptr<NakedDhcpv4Srv> srv(new NakedDhcpv4Srv(0));
@@ -318,17 +425,22 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
     // are returned when requested.
     configureRequestedOptions();
 
+    // Create a copy of the original packet by parsing its wire format.
+    // This simulates the real life scenario when we process the packet
+    // which was parsed from its wire format.
+    Pkt4Ptr received;
+    ASSERT_TRUE(createPacketFromBuffer(req, received));
     if (msg_type == DHCPDISCOVER) {
         ASSERT_NO_THROW(
-            rsp = srv->processDiscover(req);
-            );
+            rsp = srv->processDiscover(received);
+        );
 
         // Should return OFFER
         ASSERT_TRUE(rsp);
         EXPECT_EQ(DHCPOFFER, rsp->getType());
 
     } else {
-        ASSERT_NO_THROW(rsp = srv->processRequest(req););
+        ASSERT_NO_THROW(rsp = srv->processRequest(received));
 
         // Should return ACK
         ASSERT_TRUE(rsp);
@@ -336,27 +448,30 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
 
     }
 
-    messageCheck(req, rsp);
+    messageCheck(received, rsp);
 
+    // Basic options should be present when we got the lease.
+    EXPECT_TRUE(basicOptionsPresent(rsp));
     // We did not request any options so these should not be present
     // in the RSP.
-    EXPECT_FALSE(rsp->getOption(DHO_LOG_SERVERS));
-    EXPECT_FALSE(rsp->getOption(DHO_COOKIE_SERVERS));
-    EXPECT_FALSE(rsp->getOption(DHO_LPR_SERVERS));
+    EXPECT_TRUE(noRequestedOptions(rsp));
 
     // Repeat the test but request some options.
     // Add 'Parameter Request List' option.
     addPrlOption(req);
 
+    ASSERT_TRUE(createPacketFromBuffer(req, received));
+    ASSERT_TRUE(received->getOption(DHO_DHCP_PARAMETER_REQUEST_LIST));
+
     if (msg_type == DHCPDISCOVER) {
-        ASSERT_NO_THROW(rsp = srv->processDiscover(req););
+        ASSERT_NO_THROW(rsp = srv->processDiscover(received));
 
         // Should return non-NULL packet.
         ASSERT_TRUE(rsp);
         EXPECT_EQ(DHCPOFFER, rsp->getType());
 
     } else {
-        ASSERT_NO_THROW(rsp = srv->processRequest(req););
+        ASSERT_NO_THROW(rsp = srv->processRequest(received));
 
         // Should return non-NULL packet.
         ASSERT_TRUE(rsp);
@@ -364,7 +479,41 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
     }
 
     // Check that the requested options are returned.
-    optionsCheck(rsp);
+    EXPECT_TRUE(basicOptionsPresent(rsp));
+    EXPECT_TRUE(requestedOptionsPresent(rsp));
+
+    // The following part of the test will test that the NAK is sent when
+    // there is no address pool configured. In the same time, we expect
+    // that the requested options are not included in NAK message, but that
+    // they are only included when yiaddr is set to non-zero value.
+    ASSERT_NO_THROW(subnet_->delPools(Lease::TYPE_V4));
+
+    // There has been a lease allocated for the particular client. So,
+    // even though we deleted the subnet, the client would get the
+    // existing lease (not a NAK). Therefore, we have to change the chaddr
+    // in the packet so as the existing lease is not returned.
+    req->setHWAddr(1, 6, std::vector<uint8_t>(2, 6));
+    ASSERT_TRUE(createPacketFromBuffer(req, received));
+    ASSERT_TRUE(received->getOption(DHO_DHCP_PARAMETER_REQUEST_LIST));
+
+    if (msg_type == DHCPDISCOVER) {
+        ASSERT_NO_THROW(rsp = srv->processDiscover(received));
+        // Should return non-NULL packet.
+        ASSERT_TRUE(rsp);
+    } else {
+        ASSERT_NO_THROW(rsp = srv->processRequest(received));
+        // Should return non-NULL packet.
+        ASSERT_TRUE(rsp);
+    }
+    // We should get the NAK packet with yiaddr set to 0.
+    EXPECT_EQ(DHCPNAK, rsp->getType());
+    ASSERT_EQ("0.0.0.0", rsp->getYiaddr().toText());
+
+    // Make sure that none of the requested options is returned in NAK.
+    // Also options such as Routers or Subnet Mask should not be there,
+    // because lease hasn't been acquired.
+    EXPECT_TRUE(noRequestedOptions(rsp));
+    EXPECT_TRUE(noBasicOptions(rsp));
 }
 
 /// @brief This function cleans up after the test.

src/bin/dhcp4/tests/dhcp4_test_utils.h

@@ -109,10 +109,32 @@ public:
     /// @param a answer (server's message)
     void messageCheck(const Pkt4Ptr& q, const Pkt4Ptr& a);
 
-    /// @brief Check that requested options are present.
+    /// @brief Check that certain basic (always added when lease is acquired)
+    /// are present in a message.
     ///
-    /// @param pkt packet to be checked.
-    void optionsCheck(const Pkt4Ptr& pkt);
+    /// @param pkt A message to be checked.
+    /// @return Assertion result which indicates whether test passed or failed.
+    ::testing::AssertionResult basicOptionsPresent(const Pkt4Ptr& pkt);
+
+    /// @brief Check that certain basic (always added when lease is acquired)
+    /// are not present.
+    ///
+    /// @param pkt A packet to be checked.
+    /// @return Assertion result which indicates whether test passed or failed.
+    ::testing::AssertionResult noBasicOptions(const Pkt4Ptr& pkt);
+
+    /// @brief Check that certain requested options are present in the message.
+    ///
+    /// @param pkt A message to be checked.
+    /// @return Assertion result which indicates whether test passed or failed.
+    ::testing::AssertionResult requestedOptionsPresent(const Pkt4Ptr& pkt);
+
+    /// @brief Check that certain options (requested with PRL option)
+    /// are not present.
+    ///
+    /// @param pkt A packet to be checked.
+    /// @return Assertion result which indicates whether test passed or failed.
+    ::testing::AssertionResult noRequestedOptions(const Pkt4Ptr& pkt);
 
     /// @brief generates client-id option
     ///
@@ -183,6 +205,32 @@ public:
     /// @return relayed DISCOVER
     Pkt4Ptr captureRelayedDiscover();
 
+    /// @brief Create packet from output buffer of another packet.
+    ///
+    /// This function creates a packet using an output buffer from another
+    /// packet. This imitates reception of a packet from the wire. The
+    /// unpack function is then called to parse packet contents and to
+    /// create a collection of the options carried by this packet.
+    ///
+    /// This function is useful for unit tests which verify that the received
+    /// packet is parsed correctly. In those cases it is usually inappropriate
+    /// to create an instance of the packet, add options, set packet
+    /// fields and use such packet as an input to processDiscover or
+    /// processRequest function. This is because, such a packet has certain
+    /// options already initialized and there is no way to verify that they
+    /// have been initialized when packet instance was created or wire buffer
+    /// processing. By creating a packet from the buffer we guarantee that the
+    /// new packet is entirely initialized during wire data parsing.
+    ///
+    /// @param src_pkt A source packet, to be copied.
+    /// @param [out] dst_pkt A destination packet.
+    ///
+    /// @return assertion result indicating if a function completed with
+    /// success or failure.
+    static ::testing::AssertionResult
+    createPacketFromBuffer(const Pkt4Ptr& src_pkt,
+                           Pkt4Ptr& dst_pkt);
+
     /// @brief generates a DHCPv4 packet based on provided hex string
     ///
     /// @return created packet
@@ -190,6 +238,14 @@ public:
 
     /// @brief Tests if Discover or Request message is processed correctly
     ///
+    /// This test verifies that the Parameter Request List option is handled
+    /// correctly, i.e. it checks that certain options are present in the
+    /// server's response when they are requested and that they are not present
+    /// when they are not requested or NAK occurs.
+    ///
+    /// @todo We need an additional test for PRL option using real traffic
+    /// capture.
+    ///
     /// @param msg_type DHCPDISCOVER or DHCPREQUEST
     void testDiscoverRequest(const uint8_t msg_type);
 

src/lib/dhcp/option_definition.cc

@@ -124,12 +124,14 @@ OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
             return (factoryGeneric(u, type, begin, end));
 
         case OPT_UINT8_TYPE:
-            return (array_type_ ? factoryGeneric(u, type, begin, end) :
+            return (array_type_ ?
+                    factoryIntegerArray<uint8_t>(u, type, begin, end) :
                     factoryInteger<uint8_t>(u, type, getEncapsulatedSpace(),
                                             begin, end, callback));
 
         case OPT_INT8_TYPE:
-            return (array_type_ ? factoryGeneric(u, type, begin, end) :
+            return (array_type_ ?
+                    factoryIntegerArray<int8_t>(u, type, begin, end) :
                     factoryInteger<int8_t>(u, type, getEncapsulatedSpace(),
                                            begin, end, callback));
 

src/lib/dhcp/pkt4.cc

@@ -108,6 +108,10 @@ Pkt4::pack() {
         isc_throw(InvalidOperation, "Can't build Pkt4 packet. HWAddr not set.");
     }
 
+    // Clear the output buffer to make sure that consecutive calls to pack()
+    // will not result in concatenation of multiple packet copies.
+    buffer_out_.clear();
+
     try {
         size_t hw_len = hwaddr_->hwaddr_.size();
 

src/lib/dhcp/pkt4.h

@@ -72,6 +72,7 @@ public:
     /// Prepares on-wire format of message and all its options.
     /// Options must be stored in options_ field.
     /// Output buffer will be stored in buffer_out_.
+    /// The buffer_out_ is cleared before writting to the buffer.
     ///
     /// @throw InvalidOperation if packing fails
     void

src/lib/dhcp/pkt6.cc

@@ -43,7 +43,7 @@ Pkt6::Pkt6(const uint8_t* buf, uint32_t buf_len, DHCPv6Proto proto /* = UDP */)
     remote_addr_("::"),
     local_port_(0),
     remote_port_(0),
-    bufferOut_(0) {
+    buffer_out_(0) {
     data_.resize(buf_len);
     memcpy(&data_[0], buf, buf_len);
 }
@@ -58,7 +58,7 @@ Pkt6::Pkt6(uint8_t msg_type, uint32_t transid, DHCPv6Proto proto /*= UDP*/) :
     remote_addr_("::"),
     local_port_(0),
     remote_port_(0),
-    bufferOut_(0) {
+    buffer_out_(0) {
 }
 
 uint16_t Pkt6::len() {
@@ -199,6 +199,8 @@ Pkt6::pack() {
 void
 Pkt6::packUDP() {
     try {
+        // Make sure that the buffer is empty before we start writting to it.
+        buffer_out_.clear();
 
         // is this a relayed packet?
         if (!relay_info_.empty()) {
@@ -215,11 +217,11 @@ Pkt6::packUDP() {
                  relay != relay_info_.end(); ++relay) {
 
                 // build relay-forw/relay-repl header (see RFC3315, section 7)
-                bufferOut_.writeUint8(relay->msg_type_);
-                bufferOut_.writeUint8(relay->hop_count_);
-                bufferOut_.writeData(&(relay->linkaddr_.toBytes()[0]),
+                buffer_out_.writeUint8(relay->msg_type_);
+                buffer_out_.writeUint8(relay->hop_count_);
+                buffer_out_.writeData(&(relay->linkaddr_.toBytes()[0]),
                                      isc::asiolink::V6ADDRESS_LEN);
-                bufferOut_.writeData(&relay->peeraddr_.toBytes()[0],
+                buffer_out_.writeData(&relay->peeraddr_.toBytes()[0],
                                      isc::asiolink::V6ADDRESS_LEN);
 
                 // store every option in this relay scope. Usually that will be
@@ -230,28 +232,28 @@ Pkt6::packUDP() {
                 for (OptionCollection::const_iterator opt =
                          relay->options_.begin();
                      opt != relay->options_.end(); ++opt) {
-                    (opt->second)->pack(bufferOut_);
+                    (opt->second)->pack(buffer_out_);
                 }
 
                 // and include header relay-msg option. Its payload will be
                 // generated in the next iteration (if there are more relays)
                 // or outside the loop (if there are no more relays and the
                 // payload is a direct message)
-                bufferOut_.writeUint16(D6O_RELAY_MSG);
-                bufferOut_.writeUint16(relay->relay_msg_len_);
+                buffer_out_.writeUint16(D6O_RELAY_MSG);
+                buffer_out_.writeUint16(relay->relay_msg_len_);
             }
 
         }
 
         // DHCPv6 header: message-type (1 octect) + transaction id (3 octets)
-        bufferOut_.writeUint8(msg_type_);
+        buffer_out_.writeUint8(msg_type_);
         // store 3-octet transaction-id
-        bufferOut_.writeUint8( (transid_ >> 16) & 0xff );
-        bufferOut_.writeUint8( (transid_ >> 8) & 0xff );
-        bufferOut_.writeUint8( (transid_) & 0xff );
+        buffer_out_.writeUint8( (transid_ >> 16) & 0xff );
+        buffer_out_.writeUint8( (transid_ >> 8) & 0xff );
+        buffer_out_.writeUint8( (transid_) & 0xff );
 
         // the rest are options
-        LibDHCP::packOptions(bufferOut_, options_);
+        LibDHCP::packOptions(buffer_out_, options_);
     }
     catch (const Exception& e) {
        // An exception is thrown and message will be written to Logger
@@ -502,7 +504,7 @@ Pkt6::delOption(uint16_t type) {
 }
 
 void Pkt6::repack() {
-    bufferOut_.writeData(&data_[0], data_.size());
+    buffer_out_.writeData(&data_[0], data_.size());
 }
 
 void

src/lib/dhcp/pkt6.h

@@ -115,6 +115,7 @@ public:
     /// Options must be stored in options_ field.
     /// Output buffer will be stored in data_. Length
     /// will be set in data_len_.
+    /// The output buffer is cleared before new data is written to it.
     ///
     /// @throw BadValue if packet protocol is invalid, InvalidOperation
     /// if packing fails, or NotImplemented if protocol is TCP (IPv6 over TCP is
@@ -139,7 +140,7 @@ public:
     /// zero length
     ///
     /// @return reference to output buffer
-    const isc::util::OutputBuffer& getBuffer() const { return (bufferOut_); };
+    const isc::util::OutputBuffer& getBuffer() const { return (buffer_out_); };
 
     /// @brief Returns protocol of this packet (UDP or TCP).
     ///
@@ -530,7 +531,7 @@ protected:
     /// remote TCP or UDP port
     uint16_t remote_port_;
 
-    /// output buffer (used during message transmission)
+    /// Output buffer (used during message transmission)
     ///
     /// @warning This protected member is accessed by derived
     /// classes directly. One of such derived classes is
@@ -538,7 +539,7 @@ protected:
     /// behavior must be taken into consideration before making
     /// changes to this member such as access scope restriction or
     /// data format change etc.
-    isc::util::OutputBuffer bufferOut_;
+    isc::util::OutputBuffer buffer_out_;
 
     /// packet timestamp
     boost::posix_time::ptime timestamp_;

src/lib/dhcp/tests/libdhcp++_unittest.cc

@@ -706,7 +706,7 @@ TEST_F(LibDhcpTest, stdOptionDefs4) {
                                     typeid(OptionCustom));
 
     LibDhcpTest::testStdOptionDefs4(DHO_DHCP_PARAMETER_REQUEST_LIST, begin, end,
-                                    typeid(Option));
+                                    typeid(OptionUint8Array));
 
     LibDhcpTest::testStdOptionDefs4(DHO_DHCP_MESSAGE, begin, end,
                                     typeid(OptionString));

tests/tools/perfdhcp/perf_pkt6.cc

@@ -43,7 +43,7 @@ PerfPkt6::rawPack() {
                                options_,
                                getTransidOffset(),
                                getTransid(),
-                               bufferOut_));
+                               buffer_out_));
 }
 
 bool