From: Marcin Siodelski <marcin@isc.org>
Date: Mon, 3 Jun 2013 16:24:03 +0000 (+0200)
Subject: [2987] Fixed issues reported by valgrind in libdhcp++.
X-Git-Tag: bind10-1.2.0beta1-release~424^2^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=77def77e9ab95155f91dcd9506de0f8ea1001d8a;p=thirdparty%2Fkea.git

[2987] Fixed issues reported by valgrind in libdhcp++.
---

diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index 800f5e1f06..cdb01591f0 100644
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -79,13 +79,18 @@ public:
     }
 
     /// Pretends to open socket. Only records a call to this function.
+    /// This function returns fake socket descriptor (always the same).
+    /// Note that the returned value has been selected to be unique
+    /// (because real values are rather less than 255). Values greater
+    /// than 255 are not recommended because they cause warnings to be
+    /// reported by Valgrind when invoking close() on them.
     virtual int openSocket(const Iface&,
                            const isc::asiolink::IOAddress&,
                            const uint16_t,
                            const bool,
                            const bool) {
         open_socket_called_ = true;
-        return (1024);
+        return (255);
     }
 
     /// Does nothing
@@ -917,8 +922,8 @@ TEST_F(IfaceMgrTest, setPacketFilter) {
 
     // Check that openSocket function was called.
     EXPECT_TRUE(custom_packet_filter->open_socket_called_);
-    // This function always returns fake socket descriptor equal to 1024.
-    EXPECT_EQ(1024, socket1);
+    // This function always returns fake socket descriptor equal to 255.
+    EXPECT_EQ(255, socket1);
 
     // Replacing current packet filter object while there are IPv4
     // sockets open is not allowed!
diff --git a/src/lib/dhcp/tests/pkt_filter_inet_unittest.cc b/src/lib/dhcp/tests/pkt_filter_inet_unittest.cc
index a80c0645cd..5607103ce4 100644
--- a/src/lib/dhcp/tests/pkt_filter_inet_unittest.cc
+++ b/src/lib/dhcp/tests/pkt_filter_inet_unittest.cc
@@ -37,11 +37,26 @@ const size_t RECV_BUF_SIZE = 2048;
 /// its index.
 class PktFilterInetTest : public ::testing::Test {
 public:
-    PktFilterInetTest() {
+
+    /// @brief Constructor
+    ///
+    /// This constructor initializes socket_ member to the value of 0.
+    /// Explcit initialization is performed here because some of the
+    /// tests do not initialize this value. In such cases, destructor
+    /// could invoke close() on uninitialized socket descriptor which
+    /// would result in errors being reported by Valgrind. Note that
+    /// by initializing the class member to a valid socket descriptor
+    /// value (non-negative) we avoid Valgrind warning about trying to
+    /// close the invalid socket descriptor.
+    PktFilterInetTest()
+        : socket_(0) {
         // Initialize ifname_ and ifindex_.
         loInit();
     }
 
+    /// @brief Destructor
+    ///
+    /// Closes open socket (if any).
     ~PktFilterInetTest() {
         // Cleanup after each test. This guarantees
         // that the socket does not hang after a test.
diff --git a/src/lib/dhcp/tests/pkt_filter_lpf_unittest.cc b/src/lib/dhcp/tests/pkt_filter_lpf_unittest.cc
index c0724884c9..4587bd54ce 100644
--- a/src/lib/dhcp/tests/pkt_filter_lpf_unittest.cc
+++ b/src/lib/dhcp/tests/pkt_filter_lpf_unittest.cc
@@ -41,11 +41,26 @@ const size_t RECV_BUF_SIZE = 2048;
 /// its index.
 class PktFilterLPFTest : public ::testing::Test {
 public:
-    PktFilterLPFTest() {
+
+    /// @brief Constructor
+    ///
+    /// This constructor initializes socket_ member to the value of 0.
+    /// Explcit initialization is performed here because some of the
+    /// tests do not initialize this value. In such cases, destructor
+    /// could invoke close() on uninitialized socket descriptor which
+    /// would result in errors being reported by Valgrind. Note that
+    /// by initializing the class member to a valid socket descriptor
+    /// value (non-negative) we avoid Valgrind warning about trying to
+    /// close the invalid socket descriptor.
+    PktFilterLPFTest()
+        : socket_(0) {
         // Initialize ifname_ and ifindex_.
         loInit();
     }
 
+    /// @brief Destructor
+    ///
+    /// Closes open socket (if any).
     ~PktFilterLPFTest() {
         // Cleanup after each test. This guarantees
         // that the socket does not hang after a test.
diff --git a/src/lib/dhcp/tests/protocol_util_unittest.cc b/src/lib/dhcp/tests/protocol_util_unittest.cc
index eee98e6d4f..644dbf7d92 100644
--- a/src/lib/dhcp/tests/protocol_util_unittest.cc
+++ b/src/lib/dhcp/tests/protocol_util_unittest.cc
@@ -220,8 +220,10 @@ TEST(ProtocolUtilTest, writeEthernetHeader) {
     HWAddrPtr local_hw_addr(new HWAddr(src_hw_addr, 6, 1));
     ASSERT_NO_THROW(pkt->setLocalHWAddr(local_hw_addr));
 
-    // Set invalid length (7) of the hw address.
-    HWAddrPtr remote_hw_addr(new HWAddr(&std::vector<uint8_t>(1, 7)[0], 7, 1));
+    // Set invalid length (7) of the hw address. Fill it with
+    // values of 1.
+    std::vector<uint8_t> invalid_length_addr(7, 1);
+    HWAddrPtr remote_hw_addr(new HWAddr(invalid_length_addr, 1));
     ASSERT_NO_THROW(pkt->setRemoteHWAddr(remote_hw_addr));
     // HW address is too long, so it should fail.
     EXPECT_THROW(writeEthernetHeader(pkt, buf), BadValue);