[2850] Make getNamedAddress() return a std::pair

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<uint32_t*>
-                (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<uint32_t*>(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<uint32_t*>
-                (segment->getNamedAddress("zone_table_checksum"));
+            assert(result.second);
+            uint32_t* checksum = static_cast<uint32_t*>(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<ZoneTableHeader*>
-            (segment->getNamedAddress("zone_table_header"));
-        if (!header_) {
+        result = segment->getNamedAddress("zone_table_header");
+        if (result.first) {
+            assert(result.second);
+            header_ = static_cast<ZoneTableHeader*>(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<ZoneTableHeader*>
-            (segment->getNamedAddress("zone_table_header"));
-        if (!header_) {
+        result = segment->getNamedAddress("zone_table_header");
+        if (result.first) {
+            assert(result.second);
+            header_ = static_cast<ZoneTableHeader*>(result.second);
+        } else {
             isc_throw(isc::Unexpected,
                       "There is no previously saved ZoneTableHeader in a "
                       "mapped segment opened in read-only mode.");

src/lib/util/memory_segment.h

@@ -17,6 +17,8 @@
 
 #include <exceptions/exceptions.h>
 
+#include <utility>
+
 #include <stdlib.h>
 
 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<bool, void*> 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;

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<std::string, void*>::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

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.
     ///

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<void>* storage =
         impl_->base_sgmt_->find<offset_ptr<void> >(name).first;
     if (storage) {
-        return (storage->get());
+        return (NamedAddressResult(true, storage->get()));
     }
-    return (NULL);
+    return (NamedAddressResult(false, NULL));
 }
 
 bool

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.
     ///

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<void*>(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<const uint32_t*>(ptr32));
 
     // Override it.
@@ -52,20 +52,20 @@ checkSegmentNamedAddress(MemorySegment& segment, bool out_of_segment_ok) {
     const uint16_t test_val16 = 4200;
     *static_cast<uint16_t*>(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<const uint16_t*>(ptr16));
 
     // Clear it.  Then we won't be able to find it any more.
     EXPECT_TRUE(segment.clearNamedAddress("test address"));
-    EXPECT_EQ(static_cast<void*>(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<void*>(NULL),
+    EXPECT_EQ(MemorySegment::NamedAddressResult(true, NULL),
               segment.getNamedAddress("null address"));
 
     // If the underlying implementation performs explicit check against

src/lib/util/tests/memory_segment_mapped_unittest.cc

@@ -287,12 +287,14 @@ void
 checkNamedData(const std::string& name, const std::vector<uint8_t>& 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<void*>(NULL),
-              segment_->getNamedAddress("test address"));
-    EXPECT_EQ(test_val16, *static_cast<const uint16_t*>(
-                  segment_->getNamedAddress("test address")));
+    const MemorySegment::NamedAddressResult result =
+        segment_->getNamedAddress("test address");
+    ASSERT_TRUE(result.first);
+    EXPECT_EQ(test_val16, *static_cast<const uint16_t*>(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<void*>(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<const uint32_t*>(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<const uint32_t*>(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<const uint32_t*>(ptr));
+    const MemorySegment::NamedAddressResult result =
+        segment_->getNamedAddress("test address");
+    ASSERT_TRUE(result.first);
+    ASSERT_TRUE(result.second);
+    EXPECT_EQ(424242, *static_cast<const uint32_t*>(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<uint32_t*>(
-                    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<uint32_t*>(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<void*>(NULL), ptr);
+    const MemorySegment::NamedAddressResult result =
+        segment_ro.getNamedAddress("test address");
+    ASSERT_TRUE(result.first);
+    EXPECT_NE(static_cast<void*>(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