Browse Source

[master] Merge branch 'trac4252'

Marcin Siodelski 8 years ago
parent
commit
017462e92e

+ 5 - 3
src/lib/dhcp/option.h

@@ -409,9 +409,11 @@ protected:
     /// be created.
     template<typename OptionType>
     OptionPtr cloneInternal() const {
-        boost::shared_ptr<OptionType>
-            option(new OptionType(*dynamic_cast<const OptionType*>(this)));
-        return (option);
+        const OptionType* cast_this = dynamic_cast<const OptionType*>(this);
+        if (cast_this) {
+            return (boost::shared_ptr<OptionType>(new OptionType(*cast_this)));
+        }
+        return (OptionPtr());
     }
 
     /// @brief Store option's header in a buffer.

+ 16 - 0
src/lib/dhcp/tests/option_unittest.cc

@@ -9,6 +9,7 @@
 #include <dhcp/dhcp6.h>
 #include <dhcp/libdhcp++.h>
 #include <dhcp/option.h>
+#include <dhcp/option_int.h>
 #include <exceptions/exceptions.h>
 #include <util/buffer.h>
 
@@ -40,6 +41,7 @@ public:
     }
 
     using Option::unpackOptions;
+    using Option::cloneInternal;
 };
 
 class OptionTest : public ::testing::Test {
@@ -602,4 +604,18 @@ TEST_F(OptionTest, setEncapsulatedSpace) {
 
 }
 
+// This test verifies that cloneInternal returns NULL pointer if
+// non-compatible type is used as a template argument.
+// By non-compatible it is meant that the option instance doesn't
+// dynamic_cast to the type specified as template argument.
+// In our case, the NakedOption doesn't cast to OptionUint8 as the
+// latter is not derived from NakedOption.
+TEST_F(OptionTest, cloneInternal) {
+    NakedOption option;
+    OptionPtr clone;
+    // This shouldn't throw nor cause segmentation fault.
+    ASSERT_NO_THROW(clone = option.cloneInternal<OptionUint8>());
+    EXPECT_FALSE(clone);
+}
+
 }

+ 11 - 11
src/lib/dhcp/tests/pkt6_unittest.cc

@@ -23,6 +23,7 @@
 #include <boost/bind.hpp>
 #include <boost/date_time/posix_time/posix_time.hpp>
 #include <boost/scoped_ptr.hpp>
+#include <boost/pointer_cast.hpp>
 #include <util/encode/hex.h>
 #include <gtest/gtest.h>
 
@@ -145,8 +146,7 @@ TEST_F(Pkt6Test, constructor) {
 /// but we spent some time to make is less ugly than it used to be.
 ///
 /// @return pointer to Pkt6 that represents received SOLICIT
-Pkt6* capture1() {
-    Pkt6* pkt;
+Pkt6Ptr capture1() {
     uint8_t data[98];
     data[0]  = 1;
     data[1]  = 1;       data[2]  = 2;     data[3] = 3;      data[4]  = 0;
@@ -175,7 +175,7 @@ Pkt6* capture1() {
     data[93] = 6;       data[94] = 0;     data[95] = 2;     data[96] = 0;
     data[97] = 23;
 
-    pkt = new Pkt6(data, sizeof(data));
+    Pkt6Ptr pkt(new Pkt6(data, sizeof(data)));
     pkt->setRemotePort(546);
     pkt->setRemoteAddr(IOAddress("fe80::21e:8cff:fe9b:7349"));
     pkt->setLocalPort(0);
@@ -220,7 +220,7 @@ Pkt6* capture1() {
 /// The original capture was posted to dibbler users mailing list.
 ///
 /// @return created double relayed SOLICIT message
-Pkt6* capture2() {
+Pkt6Ptr capture2() {
 
     // string exported from Wireshark
     string hex_string =
@@ -238,18 +238,18 @@ Pkt6* capture2() {
     // to be OptionBuffer format)
     isc::util::encode::decodeHex(hex_string, bin);
 
-    NakedPkt6* pkt = new NakedPkt6(&bin[0], bin.size());
+    NakedPkt6Ptr pkt(new NakedPkt6(&bin[0], bin.size()));
     pkt->setRemotePort(547);
     pkt->setRemoteAddr(IOAddress("fe80::1234"));
     pkt->setLocalPort(547);
     pkt->setLocalAddr(IOAddress("ff05::1:3"));
     pkt->setIndex(2);
     pkt->setIface("eth0");
-    return (dynamic_cast<Pkt6*>(pkt));
+    return (boost::dynamic_pointer_cast<Pkt6>(pkt));
 }
 
 TEST_F(Pkt6Test, unpack_solicit1) {
-    scoped_ptr<Pkt6> sol(capture1());
+    Pkt6Ptr sol(capture1());
 
     ASSERT_NO_THROW(sol->unpack());
 
@@ -294,7 +294,7 @@ TEST_F(Pkt6Test, packUnpack) {
 TEST_F(Pkt6Test, unpackMalformed) {
     // Get a packet. We're really interested in its on-wire
     // representation only.
-    scoped_ptr<Pkt6> donor(capture1());
+    Pkt6Ptr donor(capture1());
 
     // That's our original content. It should be sane.
     OptionBuffer orig = donor->data_;
@@ -367,7 +367,7 @@ TEST_F(Pkt6Test, unpackMalformed) {
 TEST_F(Pkt6Test, unpackVendorMalformed) {
     // Get a packet. We're really interested in its on-wire
     // representation only.
-    scoped_ptr<Pkt6> donor(capture1());
+    Pkt6Ptr donor(capture1());
 
     // Add a vendor option
     OptionBuffer orig = donor->data_;
@@ -680,7 +680,7 @@ TEST_F(Pkt6Test, getName) {
 // relays can be parsed properly. See capture2() method description
 // for details regarding the packet.
 TEST_F(Pkt6Test, relayUnpack) {
-    boost::scoped_ptr<Pkt6> msg(capture2());
+    Pkt6Ptr msg(capture2());
 
     EXPECT_NO_THROW(msg->unpack());
 
@@ -870,7 +870,7 @@ TEST_F(Pkt6Test, relayPack) {
 }
 
 TEST_F(Pkt6Test, getRelayOption) {
-    NakedPkt6Ptr msg(dynamic_cast<NakedPkt6*>(capture2()));
+    NakedPkt6Ptr msg(boost::dynamic_pointer_cast<NakedPkt6>(capture2()));
     ASSERT_TRUE(msg);
 
     ASSERT_NO_THROW(msg->unpack());

+ 6 - 5
src/lib/util/csv_file.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -8,6 +8,7 @@
 #include <boost/algorithm/string/classification.hpp>
 #include <boost/algorithm/string/constants.hpp>
 #include <boost/algorithm/string/split.hpp>
+#include <algorithm>
 #include <fstream>
 #include <sstream>
 
@@ -122,7 +123,7 @@ CSVFile::addColumn(const std::string& col_name) {
 
 void
 CSVFile::addColumnInternal(const std::string& col_name) {
-    if (getColumnIndex(col_name) >= 0) {
+    if (std::find(cols_.begin(), cols_.end(), col_name) != cols_.end()) {
         isc_throw(CSVFileError, "attempt to add duplicate column '"
                   << col_name << "'");
     }
@@ -199,14 +200,14 @@ CSVFile::size() const {
     return (pos);
 }
 
-int
+size_t
 CSVFile::getColumnIndex(const std::string& col_name) const {
     for (size_t i = 0; i < cols_.size(); ++i) {
         if (cols_[i] == col_name) {
-            return (static_cast<int>(i));
+            return (i);
         }
     }
-    return (-1);
+    isc_throw(isc::OutOfRange, "column '" << col_name << "' doesn't exist");
 }
 
 std::string

+ 4 - 4
src/lib/util/csv_file.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -354,9 +354,9 @@ public:
     /// This function is exception safe.
     ///
     /// @param col_name Name of the column.
-    /// @return Index of the column or negative value if the column doesn't
-    /// exist.
-    int getColumnIndex(const std::string& col_name) const;
+    /// @return Index of the column.
+    /// @throw OutOfRange if column with such name doesn't exist.
+    size_t getColumnIndex(const std::string& col_name) const;
 
     /// @brief Returns the name of the column.
     ///

+ 4 - 1
src/lib/util/io/socketsession.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -279,6 +279,9 @@ struct SocketSessionReceiver::ReceiverImpl {
                            header_buf_(DEFAULT_HEADER_BUFLEN),
                            data_buf_(INITIAL_BUFSIZE)
     {
+        memset(&ss_local_, 0, sizeof(ss_local_));
+        memset(&ss_remote_, 0, sizeof(ss_remote_));
+
         if (setsockopt(fd_, SOL_SOCKET, SO_RCVBUF, &SOCKSESSION_BUFSIZE,
                        sizeof(SOCKSESSION_BUFSIZE)) == -1) {
             isc_throw(SocketSessionError,

+ 8 - 6
src/lib/util/versioned_csv_file.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -29,13 +29,15 @@ VersionedCSVFile::addColumn(const std::string& name,
 
 void
 VersionedCSVFile::setMinimumValidColumns(const std::string& column_name) {
-    int index = getColumnIndex(column_name);
-    if (index <  0) {
+    try {
+        int index = getColumnIndex(column_name);
+        minimum_valid_columns_ = index + 1;
+
+    } catch (...) {
         isc_throw(VersionedCSVFileError,
-                  "setMinimumValidColumns: " << column_name << " is defined");
+                  "setMinimumValidColumns: " << column_name << " is not "
+                  "defined");
     }
-
-    minimum_valid_columns_ = index + 1;
 }
 
 size_t