From: Mukund Sivaraman Date: Thu, 2 May 2013 06:45:32 +0000 (+0530) Subject: [2850] Make getNamedAddress() return a std::pair X-Git-Tag: bind10-1.2.0beta1-release~467^2~77 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=68343c845a46a9be3f38c38388ec02a8bb4ef884;p=thirdparty%2Fkea.git [2850] Make getNamedAddress() return a std::pair --- diff --git a/src/lib/datasrc/memory/zone_table_segment_mapped.cc b/src/lib/datasrc/memory/zone_table_segment_mapped.cc index 6bab7207e6..6c50700920 100644 --- a/src/lib/datasrc/memory/zone_table_segment_mapped.cc +++ b/src/lib/datasrc/memory/zone_table_segment_mapped.cc @@ -43,8 +43,11 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode, // If there is a previously opened segment, and it was // opened in read-write mode, update its checksum. mem_sgmt_->shrinkToFit(); - uint32_t* checksum = static_cast - (mem_sgmt_->getNamedAddress("zone_table_checksum")); + const MemorySegment::NamedAddressResult result = + mem_sgmt_->getNamedAddress("zone_table_checksum"); + assert(result.first); + assert(result.second); + uint32_t* checksum = static_cast(result.second); // First, clear the checksum so that getCheckSum() returns // a consistent value. *checksum = 0; @@ -86,7 +89,9 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode, (filename, MemorySegmentMapped::CREATE_ONLY)); // There must be no previously saved checksum. - if (segment->getNamedAddress("zone_table_checksum")) { + MemorySegment::NamedAddressResult result = + segment->getNamedAddress("zone_table_checksum"); + if (result.first) { isc_throw(isc::Unexpected, "There is already a saved checksum in a mapped segment " "opened in create mode."); @@ -97,7 +102,8 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode, segment->setNamedAddress("zone_table_checksum", checksum); // There must be no previously saved ZoneTableHeader. - if (segment->getNamedAddress("zone_table_header")) { + result = segment->getNamedAddress("zone_table_header"); + if (result.first) { isc_throw(isc::Unexpected, "There is already a saved ZoneTableHeader in a " "mapped segment opened in create mode."); @@ -116,11 +122,13 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode, // If there is a previously saved checksum, verify that it is // consistent. Otherwise, allocate space for a checksum (which // is saved during close). - if (segment->getNamedAddress("zone_table_checksum")) { + MemorySegment::NamedAddressResult result = + segment->getNamedAddress("zone_table_checksum"); + if (result.first) { // The segment was already shrunk when it was last // closed. Check that its checksum is consistent. - uint32_t* checksum = static_cast - (segment->getNamedAddress("zone_table_checksum")); + assert(result.second); + uint32_t* checksum = static_cast(result.second); const uint32_t saved_checksum = *checksum; // First, clear the checksum so that getCheckSum() returns // a consistent value. @@ -138,9 +146,11 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode, // If there is a previously saved ZoneTableHeader, use // it. Otherwise, allocate a new header. - header_ = static_cast - (segment->getNamedAddress("zone_table_header")); - if (!header_) { + result = segment->getNamedAddress("zone_table_header"); + if (result.first) { + assert(result.second); + header_ = static_cast(result.second); + } else { void* ptr = segment->allocate(sizeof(ZoneTableHeader)); ZoneTableHeader* new_header = new(ptr) ZoneTableHeader(ZoneTable::create(*segment, rrclass_)); @@ -153,7 +163,9 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode, case READ_ONLY: { segment.reset(new MemorySegmentMapped(filename)); // There must be a previously saved checksum. - if (!segment->getNamedAddress("zone_table_checksum")) { + MemorySegment::NamedAddressResult result = + segment->getNamedAddress("zone_table_checksum"); + if (!result.first) { isc_throw(isc::Unexpected, "There is no previously saved checksum in a " "mapped segment opened in read-only mode."); @@ -164,9 +176,11 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode, // to 0 for checksum calculation in a read-only segment. // There must be a previously saved ZoneTableHeader. - header_ = static_cast - (segment->getNamedAddress("zone_table_header")); - if (!header_) { + result = segment->getNamedAddress("zone_table_header"); + if (result.first) { + assert(result.second); + header_ = static_cast(result.second); + } else { isc_throw(isc::Unexpected, "There is no previously saved ZoneTableHeader in a " "mapped segment opened in read-only mode."); diff --git a/src/lib/util/memory_segment.h b/src/lib/util/memory_segment.h index e62c9df7b2..44b95605e9 100644 --- a/src/lib/util/memory_segment.h +++ b/src/lib/util/memory_segment.h @@ -17,6 +17,8 @@ #include +#include + #include namespace isc { @@ -176,7 +178,8 @@ public: /// as \c addr even if it wouldn't be considered to "belong to" the /// segment in its normal sense; it can be used to indicate that memory /// has not been allocated for the specified name. A subsequent call - /// to \c getNamedAddress() will return NULL for that name. + /// to \c getNamedAddress() will return std::pair (true, NULL) for + /// that name. /// /// \note Naming an address is intentionally separated from allocation /// so that, for example, one module of a program can name a memory @@ -228,6 +231,9 @@ public: return (setNamedAddressImpl(name, addr)); } + /// \brief Type definition for result returned by getNamedAddress() + typedef std::pair NamedAddressResult; + /// \brief Return the address in the segment that has the given name. /// /// This method returns the memory address in the segment corresponding @@ -245,8 +251,10 @@ public: /// /// \param name A C string of which the segment memory address is to be /// returned. Must not be NULL. - /// \return The address associated with the name, or NULL if not found. - void* getNamedAddress(const char* name) { + /// \return An std::pair containing a bool (set to true if the name + /// was found, or false otherwise) and the address associated with + /// the name (which is undefined if the name was not found). + NamedAddressResult getNamedAddress(const char* name) { // This public method implements common validation. The actual // work specific to the derived segment is delegated to the // corresponding protected method. @@ -288,7 +296,7 @@ protected: virtual bool setNamedAddressImpl(const char* name, void* addr) = 0; /// \brief Implementation of getNamedAddress beyond common validation. - virtual void* getNamedAddressImpl(const char* name) = 0; + virtual NamedAddressResult getNamedAddressImpl(const char* name) = 0; /// \brief Implementation of clearNamedAddress beyond common validation. virtual bool clearNamedAddressImpl(const char* name) = 0; diff --git a/src/lib/util/memory_segment_local.cc b/src/lib/util/memory_segment_local.cc index 81548fdb84..e68db2792b 100644 --- a/src/lib/util/memory_segment_local.cc +++ b/src/lib/util/memory_segment_local.cc @@ -51,13 +51,13 @@ MemorySegmentLocal::allMemoryDeallocated() const { return (allocated_size_ == 0 && named_addrs_.empty()); } -void* +MemorySegment::NamedAddressResult MemorySegmentLocal::getNamedAddressImpl(const char* name) { std::map::iterator found = named_addrs_.find(name); if (found != named_addrs_.end()) { - return (found->second); + return (NamedAddressResult(true, found->second)); } - return (0); + return (NamedAddressResult(false, NULL)); } bool diff --git a/src/lib/util/memory_segment_local.h b/src/lib/util/memory_segment_local.h index 1db55a0aff..90d4907c05 100644 --- a/src/lib/util/memory_segment_local.h +++ b/src/lib/util/memory_segment_local.h @@ -70,7 +70,7 @@ public: /// /// There's a small chance this method could throw std::bad_alloc. /// It should be considered a fatal error. - virtual void* getNamedAddressImpl(const char* name); + virtual NamedAddressResult getNamedAddressImpl(const char* name); /// \brief Local segment version of setNamedAddress. /// diff --git a/src/lib/util/memory_segment_mapped.cc b/src/lib/util/memory_segment_mapped.cc index e2ac944c0e..ca9c9c6cee 100644 --- a/src/lib/util/memory_segment_mapped.cc +++ b/src/lib/util/memory_segment_mapped.cc @@ -279,14 +279,14 @@ MemorySegmentMapped::allMemoryDeallocated() const { return (impl_->base_sgmt_->all_memory_deallocated()); } -void* +MemorySegment::NamedAddressResult MemorySegmentMapped::getNamedAddressImpl(const char* name) { offset_ptr* storage = impl_->base_sgmt_->find >(name).first; if (storage) { - return (storage->get()); + return (NamedAddressResult(true, storage->get())); } - return (NULL); + return (NamedAddressResult(false, NULL)); } bool diff --git a/src/lib/util/memory_segment_mapped.h b/src/lib/util/memory_segment_mapped.h index 7685e30c72..679821069d 100644 --- a/src/lib/util/memory_segment_mapped.h +++ b/src/lib/util/memory_segment_mapped.h @@ -195,7 +195,7 @@ public: /// \brief Mapped segment version of getNamedAddress. /// /// This version never throws. - virtual void* getNamedAddressImpl(const char* name); + virtual NamedAddressResult getNamedAddressImpl(const char* name); /// \brief Mapped segment version of clearNamedAddress. /// diff --git a/src/lib/util/tests/memory_segment_common_unittest.cc b/src/lib/util/tests/memory_segment_common_unittest.cc index 3810e0a3f4..4ad9c50e34 100644 --- a/src/lib/util/tests/memory_segment_common_unittest.cc +++ b/src/lib/util/tests/memory_segment_common_unittest.cc @@ -30,9 +30,8 @@ checkSegmentNamedAddress(MemorySegment& segment, bool out_of_segment_ok) { // NULL name is not allowed. EXPECT_THROW(segment.getNamedAddress(NULL), InvalidParameter); - // If the name does not exist, NULL should be returned. - EXPECT_EQ(static_cast(NULL), - segment.getNamedAddress("test address")); + // If the name does not exist, false should be returned. + EXPECT_FALSE(segment.getNamedAddress("test address").first); // Now set it void* ptr32 = segment.allocate(sizeof(uint32_t)); @@ -44,7 +43,8 @@ checkSegmentNamedAddress(MemorySegment& segment, bool out_of_segment_ok) { EXPECT_THROW(segment.setNamedAddress(NULL, ptr32), InvalidParameter); // we can now get it; the stored value should be intact. - EXPECT_EQ(ptr32, segment.getNamedAddress("test address")); + EXPECT_EQ(MemorySegment::NamedAddressResult(true, ptr32), + segment.getNamedAddress("test address")); EXPECT_EQ(test_val, *static_cast(ptr32)); // Override it. @@ -52,20 +52,20 @@ checkSegmentNamedAddress(MemorySegment& segment, bool out_of_segment_ok) { const uint16_t test_val16 = 4200; *static_cast(ptr16) = test_val16; EXPECT_FALSE(segment.setNamedAddress("test address", ptr16)); - EXPECT_EQ(ptr16, segment.getNamedAddress("test address")); + EXPECT_EQ(MemorySegment::NamedAddressResult(true, ptr16), + segment.getNamedAddress("test address")); EXPECT_EQ(test_val16, *static_cast(ptr16)); // Clear it. Then we won't be able to find it any more. EXPECT_TRUE(segment.clearNamedAddress("test address")); - EXPECT_EQ(static_cast(NULL), - segment.getNamedAddress("test address")); + EXPECT_FALSE(segment.getNamedAddress("test address").first); // duplicate attempt of clear will result in false as it doesn't exist. EXPECT_FALSE(segment.clearNamedAddress("test address")); // Setting NULL is okay. EXPECT_FALSE(segment.setNamedAddress("null address", NULL)); - EXPECT_EQ(static_cast(NULL), + EXPECT_EQ(MemorySegment::NamedAddressResult(true, NULL), segment.getNamedAddress("null address")); // If the underlying implementation performs explicit check against diff --git a/src/lib/util/tests/memory_segment_mapped_unittest.cc b/src/lib/util/tests/memory_segment_mapped_unittest.cc index 1d9979de9e..98e2a3e862 100644 --- a/src/lib/util/tests/memory_segment_mapped_unittest.cc +++ b/src/lib/util/tests/memory_segment_mapped_unittest.cc @@ -287,12 +287,14 @@ void checkNamedData(const std::string& name, const std::vector& data, MemorySegment& sgmt, bool delete_after_check = false) { - void* dp = sgmt.getNamedAddress(name.c_str()); - ASSERT_TRUE(dp); - EXPECT_EQ(0, std::memcmp(dp, &data[0], data.size())); + const MemorySegment::NamedAddressResult result = + sgmt.getNamedAddress(name.c_str()); + ASSERT_TRUE(result.first); + ASSERT_TRUE(result.second); + EXPECT_EQ(0, std::memcmp(result.second, &data[0], data.size())); if (delete_after_check) { - sgmt.deallocate(dp, data.size()); + sgmt.deallocate(result.second, data.size()); sgmt.clearNamedAddress(name.c_str()); } } @@ -309,10 +311,10 @@ TEST_F(MemorySegmentMappedTest, namedAddress) { segment_.reset(); // close it before opening another one segment_.reset(new MemorySegmentMapped(mapped_file)); - EXPECT_NE(static_cast(NULL), - segment_->getNamedAddress("test address")); - EXPECT_EQ(test_val16, *static_cast( - segment_->getNamedAddress("test address"))); + const MemorySegment::NamedAddressResult result = + segment_->getNamedAddress("test address"); + ASSERT_TRUE(result.first); + EXPECT_EQ(test_val16, *static_cast(result.second)); // try to set an unusually long name. We re-create the file so // creating the name would cause allocation failure and trigger internal @@ -323,7 +325,7 @@ TEST_F(MemorySegmentMappedTest, namedAddress) { const std::string long_name(1025, 'x'); // definitely larger than segment // setNamedAddress should return true, indicating segment has grown. EXPECT_TRUE(segment_->setNamedAddress(long_name.c_str(), NULL)); - EXPECT_EQ(static_cast(NULL), + EXPECT_EQ(MemorySegment::NamedAddressResult(true, NULL), segment_->getNamedAddress(long_name.c_str())); // Check contents pointed by named addresses survive growing and @@ -410,10 +412,12 @@ TEST_F(MemorySegmentMappedTest, multiProcess) { EXPECT_EQ(0, from_parent); MemorySegmentMapped sgmt(mapped_file); - void* ptr_child = sgmt.getNamedAddress("test address"); - EXPECT_TRUE(ptr_child); - if (ptr_child) { - const uint32_t val = *static_cast(ptr_child); + const MemorySegment::NamedAddressResult result = + sgmt.getNamedAddress("test address"); + ASSERT_TRUE(result.first); + EXPECT_TRUE(result.second); + if (result.second) { + const uint32_t val = *static_cast(result.second); EXPECT_EQ(424242, val); // tell the parent whether it succeeded. 0 means it did, // 0xff means it failed. @@ -425,9 +429,11 @@ TEST_F(MemorySegmentMappedTest, multiProcess) { // parent: open another read-only segment, then tell the child to open // its own segment. segment_.reset(new MemorySegmentMapped(mapped_file)); - ptr = segment_->getNamedAddress("test address"); - ASSERT_TRUE(ptr); - EXPECT_EQ(424242, *static_cast(ptr)); + const MemorySegment::NamedAddressResult result = + segment_->getNamedAddress("test address"); + ASSERT_TRUE(result.first); + ASSERT_TRUE(result.second); + EXPECT_EQ(424242, *static_cast(result.second)); const char some_data = 0; EXPECT_EQ(1, write(pipe_to_child.getWriteFD(), &some_data, sizeof(some_data))); @@ -477,9 +483,11 @@ TEST_F(MemorySegmentMappedTest, violateReadOnly) { if (!isc::util::unittests::runningOnValgrind()) { EXPECT_DEATH_IF_SUPPORTED({ MemorySegmentMapped segment_ro(mapped_file); - EXPECT_TRUE(segment_ro.getNamedAddress("test address")); - *static_cast( - segment_ro.getNamedAddress("test address")) = 0; + const MemorySegment::NamedAddressResult result = + segment_ro.getNamedAddress("test address"); + ASSERT_TRUE(result.first); + ASSERT_TRUE(result.second); + *static_cast(result.second) = 0; }, ""); } @@ -487,10 +495,12 @@ TEST_F(MemorySegmentMappedTest, violateReadOnly) { // attempts are prohibited. When detectable it must result in an // exception. MemorySegmentMapped segment_ro(mapped_file); - ptr = segment_ro.getNamedAddress("test address"); - EXPECT_NE(static_cast(NULL), ptr); + const MemorySegment::NamedAddressResult result = + segment_ro.getNamedAddress("test address"); + ASSERT_TRUE(result.first); + EXPECT_NE(static_cast(NULL), result.second); - EXPECT_THROW(segment_ro.deallocate(ptr, 4), MemorySegmentError); + EXPECT_THROW(segment_ro.deallocate(result.second, 4), MemorySegmentError); EXPECT_THROW(segment_ro.allocate(16), MemorySegmentError); // allocation that would otherwise require growing the segment; permission