[2850] Remove ZoneTableSegment::resetHeader()

src/lib/datasrc/memory/zone_table_segment.h

@@ -108,10 +108,9 @@ private:
 /// for the specific memory-implementation behavior.
 ///
 /// Note: At some point in the future, methods such as \c reset(),
-/// \c clear(), \c resetHeader(), \c getHeader(), \c isWritable(),
-/// \c isUsable() may become non-virtual methods. Such a change should
-/// not affect any code that uses this class, but please be aware of
-/// such plans.
+/// \c clear(), \c getHeader(), \c isWritable(), \c isUsable() may
+/// become non-virtual methods. Such a change should not affect any code
+/// that uses this class, but please be aware of such plans.
 class ZoneTableSegment {
 protected:
     /// \brief Protected constructor
@@ -338,12 +337,6 @@ public:
     /// Note that after calling \c clear(), this method will return
     /// false until the segment is reset successfully again.
     virtual bool isUsable() const = 0;
-
-    /// \brief Reset the table header address.
-    ///
-    /// This method must recalculate the \c ZoneTableHeader address, so
-    /// that it is valid when requested using the \c getHeader() method.
-    virtual void resetHeader() = 0;
 };
 
 } // namespace memory

src/lib/datasrc/memory/zone_table_segment_local.cc

@@ -60,11 +60,6 @@ ZoneTableSegmentLocal::clear()
               "should not be used.");
 }
 
-void
-ZoneTableSegmentLocal::resetHeader() {
-    // This method does not have to do anything in this implementation.
-}
-
 // After more methods' definitions are added here, it would be a good
 // idea to move getHeader() and getMemorySegment() definitions to the
 // header file.

src/lib/datasrc/memory/zone_table_segment_local.h

@@ -49,10 +49,6 @@ public:
     /// \brief Returns "local" as the implementation type.
     virtual const std::string& getImplType() const;
 
-    /// \brief This method does not have to do anything in this
-    /// implementation. It has an empty definition.
-    virtual void resetHeader();
-
     /// \brief Return the \c ZoneTableHeader for this local zone table
     /// segment.
     virtual ZoneTableHeader& getHeader();

src/lib/datasrc/memory/zone_table_segment_mapped.cc

@@ -40,8 +40,7 @@ const char* const ZONE_TABLE_HEADER_NAME = "zone_table_header";
 ZoneTableSegmentMapped::ZoneTableSegmentMapped(const RRClass& rrclass) :
     ZoneTableSegment(rrclass),
     impl_type_("mapped"),
-    rrclass_(rrclass),
-    cached_header_(NULL)
+    rrclass_(rrclass)
 {
 }
 
@@ -287,10 +286,6 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
     current_filename_ = filename;
     current_mode_ = mode;
     mem_sgmt_.reset(segment.release());
-
-    // Given what we setup above, resetHeader() must not throw at this
-    // point. If it does, all bets are off.
-    resetHeader();
 }
 
 void
@@ -322,15 +317,12 @@ ZoneTableSegmentMapped::clear() {
     }
 }
 
-void
-ZoneTableSegmentMapped::resetHeader() {
-    // We cannot perform resetHeader() lazily during getHeader() as
-    // getHeader() has to work on const objects too. So we do it here
-    // now.
-
+template<typename T>
+T*
+ZoneTableSegmentMapped::getHeaderHelper() const {
     if (!isUsable()) {
         isc_throw(isc::InvalidOperation,
-                  "resetHeader() called without calling reset() first");
+                  "getHeader() called without calling reset() first");
     }
 
     const MemorySegment::NamedAddressResult result =
@@ -341,19 +333,9 @@ ZoneTableSegmentMapped::resetHeader() {
                   "getHeader()");
     }
 
-    cached_header_ = static_cast<ZoneTableHeader*>(result.second);
-}
-
-template<typename T>
-T*
-ZoneTableSegmentMapped::getHeaderHelper() const {
-    if (!isUsable()) {
-        isc_throw(isc::InvalidOperation,
-                  "getHeader() called without calling reset() first");
-    }
-
-    assert(cached_header_);
-    return (cached_header_);
+    T* header = static_cast<ZoneTableHeader*>(result.second);
+    assert(header);
+    return (header);
 }
 
 ZoneTableHeader&

src/lib/datasrc/memory/zone_table_segment_mapped.h

@@ -50,12 +50,6 @@ public:
     /// \brief Returns "mapped" as the implementation type.
     virtual const std::string& getImplType() const;
 
-    /// \brief Resets the \c ZoneTableHeader address from the named
-    /// address in the mapped file. This method should be called once
-    /// before calls to \c getHeader() if the mapped \c MemorySegment
-    /// has grown.
-    virtual void resetHeader();
-
     /// \brief Return the \c ZoneTableHeader for this mapped zone table
     /// segment.
     ///
@@ -140,7 +134,6 @@ private:
     // Internally holds a MemorySegmentMapped. This is NULL on
     // construction, and is set by the \c reset() method.
     boost::scoped_ptr<isc::util::MemorySegmentMapped> mem_sgmt_;
-    ZoneTableHeader* cached_header_;
 };
 
 } // namespace memory

src/lib/datasrc/memory/zone_writer.cc

@@ -54,8 +54,6 @@ ZoneWriter::load() {
     }
 
     zone_data_ = load_action_(segment_.getMemorySegment());
-    segment_.resetHeader();
-
     if (!zone_data_) {
         // Bug inside load_action_.
         isc_throw(isc::InvalidOperation, "No data returned from load action");
@@ -86,8 +84,6 @@ ZoneWriter::install() {
             state_ = ZW_INSTALLED;
             zone_data_ = result.zone_data;
         } catch (const isc::util::MemorySegmentGrown&) {}
-
-        segment_.resetHeader();
     }
 }
 

src/lib/datasrc/tests/memory/zone_table_segment_mapped_unittest.cc

@@ -563,36 +563,4 @@ TEST_F(ZoneTableSegmentMappedTest, resetCreateOverCorruptedFile) {
     EXPECT_FALSE(verifyData(ztable_segment_->getMemorySegment()));
 }
 
-TEST_F(ZoneTableSegmentMappedTest, resetHeaderUninitialized) {
-    // This should throw as we haven't called reset() yet.
-    EXPECT_THROW(ztable_segment_->resetHeader(), isc::InvalidOperation);
-}
-
-TEST_F(ZoneTableSegmentMappedTest, resetHeader) {
-    // First, open an underlying mapped file in read+write mode (doesn't
-    // exist yet)
-    ztable_segment_->reset(ZoneTableSegment::READ_WRITE, config_params_);
-
-    // Check if a valid ZoneTable is found.
-    {
-        const ZoneTableHeader& header = ztable_segment_->getHeader();
-        const ZoneTable* table = header.getTable();
-        EXPECT_EQ(0, table->getZoneCount());
-    }
-
-    // Grow the segment by allocating something large.
-    EXPECT_THROW(ztable_segment_->getMemorySegment().allocate(1<<16),
-                 MemorySegmentGrown);
-
-    // Reset the header address. This should not throw now.
-    EXPECT_NO_THROW(ztable_segment_->resetHeader());
-
-    // Check if a valid ZoneTable is found.
-    {
-        const ZoneTableHeader& header = ztable_segment_->getHeader();
-        const ZoneTable* table = header.getTable();
-        EXPECT_EQ(0, table->getZoneCount());
-    }
-}
-
 } // anonymous namespace

src/lib/datasrc/tests/memory/zone_table_segment_mock.h

@@ -56,11 +56,6 @@ public:
         isc_throw(isc::NotImplemented, "clear() is not implemented");
     }
 
-    virtual void resetHeader() {
-        // This method does not have to do anything in this
-        // implementation.
-    }
-
     virtual ZoneTableHeader& getHeader() {
         return (header_);
     }

src/lib/datasrc/tests/memory/zone_table_segment_unittest.cc

@@ -88,19 +88,6 @@ TEST_F(ZoneTableSegmentTest, getHeader) {
     // const version.
     testGetHeader<const ZoneTableSegment, const ZoneTableHeader,
                   const ZoneTable>(ztable_segment_);
-
-    // This is a nop for local segments.
-    ztable_segment_->resetHeader();
-
-    // The following still behave as before after resetHeader().
-
-    // non-const version.
-    testGetHeader<ZoneTableSegment, ZoneTableHeader, ZoneTable>
-        (ztable_segment_);
-
-    // const version.
-    testGetHeader<const ZoneTableSegment, const ZoneTableHeader,
-                  const ZoneTable>(ztable_segment_);
 }
 
 TEST_F(ZoneTableSegmentTest, getMemorySegment) {

src/lib/datasrc/tests/memory/zone_writer_unittest.cc

@@ -38,25 +38,10 @@ namespace {
 
 class TestException {};
 
-class ZoneTableSegmentHelper : public ZoneTableSegmentMock {
-public:
-    ZoneTableSegmentHelper(const isc::dns::RRClass& rrclass,
-                           isc::util::MemorySegment& mem_sgmt) :
-        ZoneTableSegmentMock(rrclass, mem_sgmt),
-        reset_header_called_(false)
-    {}
-
-    virtual void resetHeader() {
-        reset_header_called_ = true;
-    }
-
-    bool reset_header_called_;
-};
-
 class ZoneWriterTest : public ::testing::Test {
 protected:
     ZoneWriterTest() :
-        segment_(new ZoneTableSegmentHelper(RRClass::IN(), mem_sgmt_)),
+        segment_(new ZoneTableSegmentMock(RRClass::IN(), mem_sgmt_)),
         writer_(new
             ZoneWriter(*segment_,
                        bind(&ZoneWriterTest::loadAction, this, _1),
@@ -71,7 +56,7 @@ protected:
         writer_.reset();
     }
     MemorySegmentMock mem_sgmt_;
-    scoped_ptr<ZoneTableSegmentHelper> segment_;
+    scoped_ptr<ZoneTableSegmentMock> segment_;
     scoped_ptr<ZoneWriter> writer_;
     bool load_called_;
     bool load_throw_;
@@ -139,18 +124,14 @@ TEST_F(ZoneWriterTest, constructForReadOnlySegment) {
 TEST_F(ZoneWriterTest, correctCall) {
     // Nothing called before we call it
     EXPECT_FALSE(load_called_);
-    EXPECT_FALSE(segment_->reset_header_called_);
 
     // Just the load gets called now
     EXPECT_NO_THROW(writer_->load());
     EXPECT_TRUE(load_called_);
-    EXPECT_TRUE(segment_->reset_header_called_);
     load_called_ = false;
-    segment_->reset_header_called_ = false;
 
     EXPECT_NO_THROW(writer_->install());
     EXPECT_FALSE(load_called_);
-    EXPECT_TRUE(segment_->reset_header_called_);
 
     // We don't check explicitly how this works, but call it to free memory. If
     // everything is freed should be checked inside the TearDown.
@@ -161,19 +142,15 @@ TEST_F(ZoneWriterTest, loadTwice) {
     // Load it the first time
     EXPECT_NO_THROW(writer_->load());
     EXPECT_TRUE(load_called_);
-    EXPECT_TRUE(segment_->reset_header_called_);
     load_called_ = false;
-    segment_->reset_header_called_ = false;
 
     // The second time, it should not be possible
     EXPECT_THROW(writer_->load(), isc::InvalidOperation);
     EXPECT_FALSE(load_called_);
-    EXPECT_FALSE(segment_->reset_header_called_);
 
     // The object should not be damaged, try installing and clearing now
     EXPECT_NO_THROW(writer_->install());
     EXPECT_FALSE(load_called_);
-    EXPECT_TRUE(segment_->reset_header_called_);
 
     // We don't check explicitly how this works, but call it to free memory. If
     // everything is freed should be checked inside the TearDown.
@@ -188,18 +165,15 @@ TEST_F(ZoneWriterTest, loadLater) {
     EXPECT_NO_THROW(writer_->install());
     // Reset so we see nothing is called now
     load_called_ = false;
-    segment_->reset_header_called_ = false;
 
     EXPECT_THROW(writer_->load(), isc::InvalidOperation);
     EXPECT_FALSE(load_called_);
-    EXPECT_FALSE(segment_->reset_header_called_);
 
     // Cleanup and try loading again. Still shouldn't work.
     EXPECT_NO_THROW(writer_->cleanup());
 
     EXPECT_THROW(writer_->load(), isc::InvalidOperation);
     EXPECT_FALSE(load_called_);
-    EXPECT_FALSE(segment_->reset_header_called_);
 }
 
 // Try calling install at various bad times