BIND 10 trac1238, updated. 86a4ce45115dab4d3978c36dd2dbe07edcac02ac [1238] Changes after review: openSocket*() now returns int

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Dec 7 12:16:45 UTC 2011


The branch, trac1238 has been updated
       via  86a4ce45115dab4d3978c36dd2dbe07edcac02ac (commit)
       via  14a64484a3159a142f1b83a9830ac389a52f6a35 (commit)
       via  146203239c50d2a00069986944d4ec168f17b31f (commit)
      from  660cf410c0fb41587b977df992879f5dff934c19 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 86a4ce45115dab4d3978c36dd2dbe07edcac02ac
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Dec 7 13:15:46 2011 +0100

    [1238] Changes after review: openSocket*() now returns int

commit 14a64484a3159a142f1b83a9830ac389a52f6a35
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Dec 7 13:09:28 2011 +0100

    [1238] Memory leak in Option test fixed.

commit 146203239c50d2a00069986944d4ec168f17b31f
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Dec 7 12:58:26 2011 +0100

    [1238] Pkt4 now stores input data (a workaround for limitation of InputBuffer)

-----------------------------------------------------------------------

Summary of changes:
 src/bin/dhcp6/iface_mgr.cc                |   15 +++------
 src/bin/dhcp6/iface_mgr.h                 |    8 ++--
 src/bin/dhcp6/tests/dhcp6_srv_unittest.cc |    8 ++---
 src/lib/dhcp/pkt4.cc                      |   47 ++++++++++++++++-------------
 src/lib/dhcp/pkt4.h                       |   11 ++++---
 src/lib/dhcp/tests/option_unittest.cc     |    2 +
 6 files changed, 46 insertions(+), 45 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/iface_mgr.cc b/src/bin/dhcp6/iface_mgr.cc
index b28b9f0..60dac63 100644
--- a/src/bin/dhcp6/iface_mgr.cc
+++ b/src/bin/dhcp6/iface_mgr.cc
@@ -87,11 +87,7 @@ bool IfaceMgr::Iface::delAddress(const isc::asiolink::IOAddress& addr) {
     // of the same address added, we are in deep problems anyway.
     size_t size = addrs_.size();
     addrs_.erase(remove(addrs_.begin(), addrs_.end(), addr), addrs_.end());
-    if (addrs_.size() < size) {
-        return (true);
-    } else {
-        return (false);
-    }
+    return (addrs_.size() < size);
 }
 
 bool IfaceMgr::Iface::delSocket(uint16_t sockfd) {
@@ -274,9 +270,8 @@ IfaceMgr::getIface(const std::string& ifname) {
     return (NULL); // not found
 }
 
-uint16_t
-IfaceMgr::openSocket(const std::string& ifname,
-                     const IOAddress& addr,
+int
+IfaceMgr::openSocket(const std::string& ifname, const IOAddress& addr,
                      int port) {
     Iface* iface = getIface(ifname);
     if (!iface) {
@@ -293,7 +288,7 @@ IfaceMgr::openSocket(const std::string& ifname,
     }
 }
 
-uint16_t
+int
 IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
 
     cout << "Creating UDP4 socket on " << iface.getFullName()
@@ -335,7 +330,7 @@ IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
     return (sock);
 }
 
-uint16_t
+int
 IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
 
     cout << "Creating UDP6 socket on " << iface.getFullName()
diff --git a/src/bin/dhcp6/iface_mgr.h b/src/bin/dhcp6/iface_mgr.h
index a8d70d9..0aa2592 100644
--- a/src/bin/dhcp6/iface_mgr.h
+++ b/src/bin/dhcp6/iface_mgr.h
@@ -291,8 +291,8 @@ public:
     ///
     /// @return socket descriptor, if socket creation, binding and multicast
     /// group join were all successful.
-    uint16_t openSocket(const std::string& ifname,
-                        const isc::asiolink::IOAddress& addr, int port);
+    int openSocket(const std::string& ifname,
+                   const isc::asiolink::IOAddress& addr, int port);
 
     /// Opens IPv6 sockets on detected interfaces.
     ///
@@ -328,7 +328,7 @@ protected:
     /// @param port a port that created socket should be bound to
     ///
     /// @return socket descriptor
-    uint16_t openSocket4(Iface& iface, const isc::asiolink::IOAddress& addr, int port);
+    int openSocket4(Iface& iface, const isc::asiolink::IOAddress& addr, int port);
 
     /// @brief Opens IPv6 socket.
     ///
@@ -341,7 +341,7 @@ protected:
     /// @param port a port that created socket should be bound to
     ///
     /// @return socket descriptor
-    uint16_t openSocket6(Iface& iface, const isc::asiolink::IOAddress& addr, int port);
+    int openSocket6(Iface& iface, const isc::asiolink::IOAddress& addr, int port);
 
     /// @brief Adds an interface to list of known interfaces.
     ///
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index d5f2b95..d305aa0 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -57,15 +57,13 @@ TEST_F(Dhcpv6SrvTest, basic) {
     // interfaces.txt instead. It will pretend to have detected
     // fe80::1234 link-local address on eth0 interface. Obviously
     // an attempt to bind this socket will fail.
-    Dhcpv6Srv * srv = 0;
+    Dhcpv6Srv* srv = 0;
     ASSERT_NO_THROW( {
         // open an unpriviledged port
         srv = new Dhcpv6Srv(DHCP6_SERVER_PORT + 10000);
     });
-    if (srv) {
-        delete srv;
-    }
 
+    delete srv;
 }
 
 TEST_F(Dhcpv6SrvTest, Solicit_basic) {
@@ -75,7 +73,7 @@ TEST_F(Dhcpv6SrvTest, Solicit_basic) {
     // a dummy content for client-id
     boost::shared_array<uint8_t> clntDuid(new uint8_t[32]);
     for (int i = 0; i < 32; i++)
-        clntDuid[i] = 100+i;
+        clntDuid[i] = 100 + i;
 
     boost::shared_ptr<Pkt6> sol =
         boost::shared_ptr<Pkt6>(new Pkt6(DHCPV6_SOLICIT,
diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc
index ba07a10..14aa9e8 100644
--- a/src/lib/dhcp/pkt4.cc
+++ b/src/lib/dhcp/pkt4.cc
@@ -47,7 +47,6 @@ Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
       yiaddr_(DEFAULT_ADDRESS),
       siaddr_(DEFAULT_ADDRESS),
       giaddr_(DEFAULT_ADDRESS),
-      bufferIn_(NULL, 0), // not used, this is TX packet
       bufferOut_(DHCPV4_PKT_HDR_LEN),
       msg_type_(msg_type)
 {
@@ -73,7 +72,6 @@ Pkt4::Pkt4(const uint8_t* data, size_t len)
       yiaddr_(DEFAULT_ADDRESS),
       siaddr_(DEFAULT_ADDRESS),
       giaddr_(DEFAULT_ADDRESS),
-      bufferIn_(data, len),
       bufferOut_(0), // not used, this is RX packet
       msg_type_(DHCPDISCOVER)
 {
@@ -82,6 +80,9 @@ Pkt4::Pkt4(const uint8_t* data, size_t len)
                   << " received, at least " << DHCPV4_PKT_HDR_LEN
                   << "is expected");
     }
+
+    data_.resize(len);
+    memcpy(&data_[0], data, len);
 }
 
 size_t
@@ -121,31 +122,35 @@ Pkt4::pack() {
 }
 bool
 Pkt4::unpack() {
-    if (bufferIn_.getLength()<DHCPV4_PKT_HDR_LEN) {
+
+    // input buffer (used during message reception)
+    isc::util::InputBuffer bufferIn(&data_[0], data_.size());
+
+    if (bufferIn.getLength()<DHCPV4_PKT_HDR_LEN) {
         isc_throw(OutOfRange, "Received truncated DHCPv4 packet (len="
-                  << bufferIn_.getLength() << " received, at least "
+                  << bufferIn.getLength() << " received, at least "
                   << DHCPV4_PKT_HDR_LEN << "is expected");
     }
 
-    op_ = bufferIn_.readUint8();
-    htype_ = bufferIn_.readUint8();
-    hlen_ = bufferIn_.readUint8();
-    hops_ = bufferIn_.readUint8();
-    transid_ = bufferIn_.readUint32();
-    secs_ = bufferIn_.readUint16();
-    flags_ = bufferIn_.readUint16();
-    ciaddr_ = IOAddress(bufferIn_.readUint32());
-    yiaddr_ = IOAddress(bufferIn_.readUint32());
-    siaddr_ = IOAddress(bufferIn_.readUint32());
-    giaddr_ = IOAddress(bufferIn_.readUint32());
-    bufferIn_.readData(chaddr_, MAX_CHADDR_LEN);
-    bufferIn_.readData(sname_, MAX_SNAME_LEN);
-    bufferIn_.readData(file_, MAX_FILE_LEN);
-
-    size_t opts_len = bufferIn_.getLength() - bufferIn_.getPosition();
+    op_ = bufferIn.readUint8();
+    htype_ = bufferIn.readUint8();
+    hlen_ = bufferIn.readUint8();
+    hops_ = bufferIn.readUint8();
+    transid_ = bufferIn.readUint32();
+    secs_ = bufferIn.readUint16();
+    flags_ = bufferIn.readUint16();
+    ciaddr_ = IOAddress(bufferIn.readUint32());
+    yiaddr_ = IOAddress(bufferIn.readUint32());
+    siaddr_ = IOAddress(bufferIn.readUint32());
+    giaddr_ = IOAddress(bufferIn.readUint32());
+    bufferIn.readData(chaddr_, MAX_CHADDR_LEN);
+    bufferIn.readData(sname_, MAX_SNAME_LEN);
+    bufferIn.readData(file_, MAX_FILE_LEN);
+
+    size_t opts_len = bufferIn.getLength() - bufferIn.getPosition();
     vector<uint8_t> optsBuffer;
     // fist use of readVector
-    bufferIn_.readVector(optsBuffer, opts_len);
+    bufferIn.readVector(optsBuffer, opts_len);
     LibDHCP::unpackOptions4(optsBuffer, options_);
 
     return (true);
diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h
index db26fa5..189d95d 100644
--- a/src/lib/dhcp/pkt4.h
+++ b/src/lib/dhcp/pkt4.h
@@ -396,14 +396,15 @@ protected:
 
     // end of real DHCPv4 fields
 
-    /// input buffer (used during message reception)
-    /// Note that it must be modifiable as hooks can modify incoming buffer),
-    /// thus OutputBuffer, not InputBuffer
-    isc::util::InputBuffer bufferIn_;
-
     /// output buffer (used during message
     isc::util::OutputBuffer bufferOut_;
 
+    // that's the data of input buffer used in RX packet. Note that
+    // InputBuffer does not store the data itself, but just expects that
+    // data will be valid for the whole life of InputBuffer. Therefore we
+    // need to keep the data around.
+    std::vector<uint8_t> data_;
+
     /// message type (e.g. 1=DHCPDISCOVER)
     /// TODO: this will eventually be replaced with DHCP Message Type
     /// option (option 53)
diff --git a/src/lib/dhcp/tests/option_unittest.cc b/src/lib/dhcp/tests/option_unittest.cc
index db3ee3b..66dce8f 100644
--- a/src/lib/dhcp/tests/option_unittest.cc
+++ b/src/lib/dhcp/tests/option_unittest.cc
@@ -402,6 +402,8 @@ TEST_F(OptionTest, v6_addgetdel) {
 
     // let's try to delete - should fail
     EXPECT_TRUE(false ==  parent->delOption(2));
+
+    delete parent;
 }
 
 }




More information about the bind10-changes mailing list