BIND 10 trac2268, updated. ec2aecacc46f071cc6479881418825920e8f8f50 [2268] Fix a leak upon exception in InMemoryClient's constructor

BIND 10 source code commits bind10-changes at lists.isc.org
Sun Oct 7 17:59:02 UTC 2012


The branch, trac2268 has been updated
       via  ec2aecacc46f071cc6479881418825920e8f8f50 (commit)
       via  83a27d452c13b468d2103b0314ecf2bae17cd6b1 (commit)
       via  ece07281abf99ae142b20b4aead428e3333121a9 (commit)
       via  d05029362843411c46c6d8d278fcf17ba9d8d32a (commit)
       via  ef49adadf64f56558c75d34483895e1e8d8bcf66 (commit)
      from  a475756bf185c9e364ed00e245e307789e22fed3 (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 ec2aecacc46f071cc6479881418825920e8f8f50
Author: Mukund Sivaraman <muks at isc.org>
Date:   Sun Oct 7 23:25:55 2012 +0530

    [2268] Fix a leak upon exception in InMemoryClient's constructor

commit 83a27d452c13b468d2103b0314ecf2bae17cd6b1
Author: Mukund Sivaraman <muks at isc.org>
Date:   Sun Oct 7 23:02:57 2012 +0530

    [2268] Remove redundant MemorySegmentTest definition

commit ece07281abf99ae142b20b4aead428e3333121a9
Author: Mukund Sivaraman <muks at isc.org>
Date:   Sun Oct 7 22:59:27 2012 +0530

    [2268] Move constructor and destructor to top of file

commit d05029362843411c46c6d8d278fcf17ba9d8d32a
Author: Mukund Sivaraman <muks at isc.org>
Date:   Sun Oct 7 21:18:58 2012 +0530

    [2268] Update ZoneDataUpdater API doc

commit ef49adadf64f56558c75d34483895e1e8d8bcf66
Author: Mukund Sivaraman <muks at isc.org>
Date:   Sun Oct 7 21:18:38 2012 +0530

    [2268] Move EmptyZone exception to InMemoryClient class

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

Summary of changes:
 src/lib/datasrc/memory/memory_client.cc            |   39 +++++++------
 src/lib/datasrc/memory/memory_client.h             |   10 ++++
 src/lib/datasrc/memory/zone_data_updater.h         |   60 ++++++++++++++++----
 .../datasrc/tests/memory/memory_client_unittest.cc |   11 ++--
 .../datasrc/tests/memory/zone_table_unittest.cc    |   25 +-------
 5 files changed, 91 insertions(+), 54 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/memory_client.cc b/src/lib/datasrc/memory/memory_client.cc
index de1b91d..552c5e7 100644
--- a/src/lib/datasrc/memory/memory_client.cc
+++ b/src/lib/datasrc/memory/memory_client.cc
@@ -62,6 +62,27 @@ public:
     }
 };
 
+InMemoryClient::InMemoryClient(util::MemorySegment& mem_sgmt,
+                               RRClass rrclass) :
+    mem_sgmt_(mem_sgmt),
+    rrclass_(rrclass),
+    zone_count_(0)
+{
+    SegmentObjectHolder<ZoneTable, RRClass> holder(
+        mem_sgmt_, ZoneTable::create(mem_sgmt_, rrclass), rrclass_);
+
+    file_name_tree_ = FileNameTree::create(mem_sgmt_, false);
+
+    zone_table_ = holder.release();
+}
+
+InMemoryClient::~InMemoryClient() {
+    FileNameDeleter deleter;
+    FileNameTree::destroy(mem_sgmt_, file_name_tree_, deleter);
+
+    ZoneTable::destroy(mem_sgmt_, zone_table_, rrclass_);
+}
+
 result::Result
 InMemoryClient::load(const Name& zone_name,
                      const string& filename,
@@ -92,7 +113,7 @@ InMemoryClient::load(const Name& zone_name,
     // an SOA RR. This condition should be avoided, and hence load()
     // should throw when an empty zone is loaded.
     if (RdataSet::find(set, RRType::SOA()) == NULL) {
-        isc_throw(ZoneDataUpdater::EmptyZone,
+        isc_throw(EmptyZone,
                   "Won't create an empty zone for: " << zone_name);
     }
 
@@ -161,22 +182,6 @@ generateRRsetFromIterator(ZoneIterator* iterator,
 }
 }
 
-InMemoryClient::InMemoryClient(util::MemorySegment& mem_sgmt,
-                               RRClass rrclass) :
-    mem_sgmt_(mem_sgmt),
-    rrclass_(rrclass),
-    zone_count_(0),
-    zone_table_(ZoneTable::create(mem_sgmt_, rrclass)),
-    file_name_tree_(FileNameTree::create(mem_sgmt_, false))
-{}
-
-InMemoryClient::~InMemoryClient() {
-    FileNameDeleter deleter;
-    FileNameTree::destroy(mem_sgmt_, file_name_tree_, deleter);
-
-    ZoneTable::destroy(mem_sgmt_, zone_table_, rrclass_);
-}
-
 RRClass
 InMemoryClient::getClass() const {
     return (rrclass_);
diff --git a/src/lib/datasrc/memory/memory_client.h b/src/lib/datasrc/memory/memory_client.h
index 262cbb7..edd3837 100644
--- a/src/lib/datasrc/memory/memory_client.h
+++ b/src/lib/datasrc/memory/memory_client.h
@@ -87,6 +87,16 @@ public:
     /// \return The number of zones stored in the client.
     virtual unsigned int getZoneCount() const;
 
+    /// \brief Zone is empty exception.
+    ///
+    /// This is thrown if we have an empty zone created as a result of
+    /// load().
+    struct EmptyZone : public InvalidParameter {
+        EmptyZone(const char* file, size_t line, const char* what) :
+            InvalidParameter(file, line, what)
+        {}
+    };
+
     /// \brief Load zone from masterfile.
     ///
     /// This loads data from masterfile specified by filename. It replaces
diff --git a/src/lib/datasrc/memory/zone_data_updater.h b/src/lib/datasrc/memory/zone_data_updater.h
index 1c00aef..6ccad26 100644
--- a/src/lib/datasrc/memory/zone_data_updater.h
+++ b/src/lib/datasrc/memory/zone_data_updater.h
@@ -30,8 +30,38 @@ namespace isc {
 namespace datasrc {
 namespace memory {
 
+/// \brief A helper class to add records to a zone.
+///
+/// This class provides an \c add() method that can be used to add
+/// RRsets to a ZoneData instance. The RRsets are first validated for
+/// correctness and consistency, and their data is made into RdataSets
+/// which are added to the ZoneData for the zone.
+///
+/// The way to use this is to make a ZoneDataUpdater instance, and call
+/// add() on it as follows:
+///
+/// ZoneDataUpdater updater(mem_sgmt, rrclass, zone_origin_name, zone_data);
+/// ConstRRsetPtr rrset;
+/// updater.add(rrset, ConstRRsetPtr());
+///
+/// We enforce that instances are non-copyable as it's pointless to make
+/// copies.
 class ZoneDataUpdater : boost::noncopyable {
 public:
+    ///
+    /// \name Constructors and Destructor.
+    ///
+    //@{
+
+    /// The constructor.
+    ///
+    /// \throw none
+    ///
+    /// \param mem_sgmt The memory segment used for the zone data.
+    /// \param rrclass The RRclass of the zone data.
+    /// \param zone_name The Name of the zone under which records will be added.
+    //  \param zone_data The ZoneData object which is populated with
+    //                   record data.
     ZoneDataUpdater(util::MemorySegment& mem_sgmt,
                     isc::dns::RRClass rrclass,
                     const isc::dns::Name& zone_name,
@@ -50,23 +80,14 @@ public:
 
     //@}
 
-    /// This is thrown if the provided RRset parameter is NULL.
+    /// This is thrown if the provided RRset parameter passed to \c
+    /// add() is NULL.
     struct NullRRset : public InvalidParameter {
         NullRRset(const char* file, size_t line, const char* what) :
             InvalidParameter(file, line, what)
         {}
     };
 
-    /// \brief Zone is empty exception.
-    ///
-    /// This is thrown if we have an empty zone created as a result of
-    /// load().
-    struct EmptyZone : public InvalidParameter {
-        EmptyZone(const char* file, size_t line, const char* what) :
-            InvalidParameter(file, line, what)
-        {}
-    };
-
     /// \brief General failure exception for \c add().
     ///
     /// This is thrown against general error cases in adding an RRset
@@ -82,6 +103,23 @@ public:
         {}
     };
 
+    /// \brief Add an RRset to the zone.
+    ///
+    /// This is the core method of this class. It is used to add an
+    /// RRset to the ZoneData associated with this object. The RRset is
+    /// first validated for correctness and consistency with the rest of
+    /// the records in the zone, and then an RdataSet is created and
+    /// populated with the record data and added to the ZoneData for the
+    /// name in the RRset.
+    ///
+    /// This method throws an \c NullRRset exception (see above) if
+    /// \c rrset is empty. It throws \c AddError if any of a variety of
+    /// validation checks fail for the \c rrset and its associated
+    /// \c sig_rrset.
+    ///
+    /// \param rrset The RRset to be added.
+    /// \param sig_rrset An associated RRSIG RRset for the \c rrset. It
+    ///                  can be empty if there is no RRSIG for the \c rrset.
     void add(const isc::dns::ConstRRsetPtr& rrset,
              const isc::dns::ConstRRsetPtr& sig_rrset);
 
diff --git a/src/lib/datasrc/tests/memory/memory_client_unittest.cc b/src/lib/datasrc/tests/memory/memory_client_unittest.cc
index c078105..4a34bad 100644
--- a/src/lib/datasrc/tests/memory/memory_client_unittest.cc
+++ b/src/lib/datasrc/tests/memory/memory_client_unittest.cc
@@ -189,7 +189,7 @@ TEST_F(MemoryClientTest, loadEmptyZoneFileThrows) {
 
     EXPECT_THROW(client_->load(Name("."),
                                TEST_DATA_DIR "/empty.zone"),
-                 ZoneDataUpdater::EmptyZone);
+                 InMemoryClient::EmptyZone);
 
     EXPECT_EQ(0, client_->getZoneCount());
 
@@ -261,10 +261,13 @@ TEST_F(MemoryClientTest, loadMemoryAllocationFailures) {
     // Just to check that things get cleaned up
 
     for (int i = 1; i < 16; i++) {
+        SCOPED_TRACE("For throw count = " + i);
         mem_sgmt_.setThrowCount(i);
-        EXPECT_THROW(client_->load(Name("example.org"),
-                                   TEST_DATA_DIR "/example.org.zone"),
-                     std::bad_alloc);
+        EXPECT_THROW({
+            InMemoryClient client2(mem_sgmt_, zclass_);
+            client2.load(Name("example.org"),
+                         TEST_DATA_DIR "/example.org.zone");
+        }, std::bad_alloc);
     }
     // Teardown checks for memory segment leaks
 }
diff --git a/src/lib/datasrc/tests/memory/zone_table_unittest.cc b/src/lib/datasrc/tests/memory/zone_table_unittest.cc
index 359df49..a8ee614 100644
--- a/src/lib/datasrc/tests/memory/zone_table_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_table_unittest.cc
@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include "memory_segment_test.h"
+
 #include <exceptions/exceptions.h>
 
 #include <util/memory_segment_local.h>
@@ -32,27 +34,6 @@ using namespace isc::datasrc;
 using namespace isc::datasrc::memory;
 
 namespace {
-// Memory segment specified for tests.  It normally behaves like a "local"
-// memory segment.  If "throw count" is set to non 0 via setThrowCount(),
-// it continues the normal behavior up to the specified number of calls to
-// allocate(), and throws an exception at the next call.
-class TestMemorySegment : public isc::util::MemorySegmentLocal {
-public:
-    TestMemorySegment() : throw_count_(0) {}
-    virtual void* allocate(size_t size) {
-        if (throw_count_ > 0) {
-            if (--throw_count_ == 0) {
-                throw std::bad_alloc();
-            }
-        }
-        return (isc::util::MemorySegmentLocal::allocate(size));
-    }
-    void setThrowCount(size_t count) { throw_count_ = count; }
-
-private:
-    size_t throw_count_;
-};
-
 class ZoneTableTest : public ::testing::Test {
 protected:
     ZoneTableTest() : zclass_(RRClass::IN()),
@@ -73,7 +54,7 @@ protected:
     }
     const RRClass zclass_;
     const Name zname1, zname2, zname3;
-    TestMemorySegment mem_sgmt_;
+    test::MemorySegmentTest mem_sgmt_;
     ZoneTable* zone_table;
 };
 



More information about the bind10-changes mailing list