[2831] handle NULL name values for xxxNamedAddress explicitly.

src/lib/util/memory_segment.h

@@ -196,12 +196,22 @@ public:
     ///
     /// \throw std::bad_alloc Allocation of a segment space for the given name
     /// failed.
+    /// \throw InvalidParameter name is NULL.
     ///
-    /// \param name A C string to be associated with \c addr.
+    /// \param name A C string to be associated with \c addr. Must not be NULL.
     /// \param addr A memory address returned by a prior call to \c allocate.
     /// \return true if the internal segment has grown to allocate space for
     /// the name; false otherwise (see above).
-    virtual bool setNamedAddress(const char* name, void* addr) = 0;
+    bool setNamedAddress(const char* name, void* addr) {
+        // This public method implements common validation.  The actual
+        // work specific to the derived segment is delegated to the
+        // corresponding protected method.
+        if (!name) {
+            isc_throw(InvalidParameter,
+                      "NULL name is given to setNamedAddress");
+        }
+        return (setNamedAddressImpl(name, addr));
+    }
 
     /// \brief Return the address in the segment that has the given name.
     ///
@@ -216,10 +226,21 @@ public:
     /// API doesn't guarantee that property.  In general, if this method
     /// throws it should be considered a fatal condition.
     ///
+    /// \throw InvalidParameter name is NULL.
+    ///
     /// \param name A C string of which the segment memory address is to be
-    /// returned.
+    /// returned.  Must not be NULL.
     /// \return The address associated with the name, or NULL if not found.
-    virtual void* getNamedAddress(const char* name) = 0;
+    void* 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.
+        if (!name) {
+            isc_throw(InvalidParameter,
+                      "NULL name is given to getNamedAddress");
+        }
+        return (getNamedAddressImpl(name));
+    }
 
     /// \brief Delete a name previously associated with a segment address.
     ///
@@ -230,9 +251,30 @@ public:
     ///
     /// See \c getNamedAddress() about exception consideration.
     ///
+    /// \throw InvalidParameter name is NULL.
+    ///
     /// \param name A C string of which the segment memory address is to be
-    /// deleted.
-    virtual bool clearNamedAddress(const char* name) = 0;
+    /// deleted. Must not be NULL.
+    bool clearNamedAddress(const char* name) {
+        // This public method implements common validation.  The actual
+        // work specific to the derived segment is delegated to the
+        // corresponding protected method.
+        if (!name) {
+            isc_throw(InvalidParameter,
+                      "NULL name is given to clearNamedAddress");
+        }
+        return (clearNamedAddressImpl(name));
+    }
+
+protected:
+    /// \brief Implementation of setNamedAddress beyond common validation.
+    virtual bool setNamedAddressImpl(const char* name, void* addr) = 0;
+
+    /// \brief Implementation of getNamedAddress beyond common validation.
+    virtual void* getNamedAddressImpl(const char* name) = 0;
+
+    /// \brief Implementation of clearNamedAddress beyond common validation.
+    virtual bool clearNamedAddressImpl(const char* name) = 0;
 };
 
 } // namespace util

src/lib/util/memory_segment_local.cc

@@ -52,7 +52,7 @@ MemorySegmentLocal::allMemoryDeallocated() const {
 }
 
 void*
-MemorySegmentLocal::getNamedAddress(const char* name) {
+MemorySegmentLocal::getNamedAddressImpl(const char* name) {
     std::map<std::string, void*>::iterator found = named_addrs_.find(name);
     if (found != named_addrs_.end()) {
         return (found->second);
@@ -61,13 +61,13 @@ MemorySegmentLocal::getNamedAddress(const char* name) {
 }
 
 bool
-MemorySegmentLocal::setNamedAddress(const char* name, void* addr) {
+MemorySegmentLocal::setNamedAddressImpl(const char* name, void* addr) {
     named_addrs_[name] = addr;
     return (false);
 }
 
 bool
-MemorySegmentLocal::clearNamedAddress(const char* name) {
+MemorySegmentLocal::clearNamedAddressImpl(const char* name) {
     const size_t n_erased = named_addrs_.erase(name);
     return (n_erased != 0);
 }

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* getNamedAddress(const char* name);
+    virtual void* getNamedAddressImpl(const char* name);
 
     /// \brief Local segment version of setNamedAddress.
     ///
@@ -81,13 +81,13 @@ public:
     /// application should expect a return value of \c true unless it knows
     /// the memory segment class is \c MemorySegmentLocal and needs to
     /// exploit the fact).
-    virtual bool setNamedAddress(const char* name, void* addr);
+    virtual bool setNamedAddressImpl(const char* name, void* addr);
 
     /// \brief Local segment version of clearNamedAddress.
     ///
     /// There's a small chance this method could throw std::bad_alloc.
     /// It should be considered a fatal error.
-    virtual bool clearNamedAddress(const char* name);
+    virtual bool clearNamedAddressImpl(const char* name);
 
 private:
     // allocated_size_ can underflow, wrap around to max size_t (which

src/lib/util/memory_segment_mapped.cc

@@ -174,7 +174,7 @@ MemorySegmentMapped::allMemoryDeallocated() const {
 }
 
 void*
-MemorySegmentMapped::getNamedAddress(const char* name) {
+MemorySegmentMapped::getNamedAddressImpl(const char* name) {
     offset_ptr<void>* storage =
         impl_->base_sgmt_->find<offset_ptr<void> >(name).first;
     if (storage) {
@@ -184,7 +184,7 @@ MemorySegmentMapped::getNamedAddress(const char* name) {
 }
 
 bool
-MemorySegmentMapped::setNamedAddress(const char* name, void* addr) {
+MemorySegmentMapped::setNamedAddressImpl(const char* name, void* addr) {
     if (impl_->read_only_) {
         isc_throw(InvalidOperation, "setNamedAddress on read-only segment");
     }
@@ -209,7 +209,7 @@ MemorySegmentMapped::setNamedAddress(const char* name, void* addr) {
 }
 
 bool
-MemorySegmentMapped::clearNamedAddress(const char* name) {
+MemorySegmentMapped::clearNamedAddressImpl(const char* name) {
     if (impl_->read_only_) {
         isc_throw(InvalidOperation, "clearNamedAddress on read-only segment");
     }

src/lib/util/memory_segment_mapped.h

@@ -139,17 +139,17 @@ public:
     ///
     /// This method cannot be called if the segment object is created in the
     /// read-only mode; in that case InvalidOperation will be thrown.
-    virtual bool setNamedAddress(const char* name, void* addr);
+    virtual bool setNamedAddressImpl(const char* name, void* addr);
 
     /// \brief Mapped segment version of getNamedAddress.
     ///
     /// This version never throws.
-    virtual void* getNamedAddress(const char* name);
+    virtual void* getNamedAddressImpl(const char* name);
 
     /// \brief Mapped segment version of clearNamedAddress.
     ///
     /// This version never throws.
-    virtual bool clearNamedAddress(const char* name);
+    virtual bool clearNamedAddressImpl(const char* name);
 
     /// \brief Shrink the underlying mapped segment to actually used size.
     ///

src/lib/util/tests/memory_segment_common_unittest.cc

@@ -14,6 +14,8 @@
 
 #include <util/memory_segment.h>
 
+#include <exceptions/exceptions.h>
+
 #include <gtest/gtest.h>
 
 #include <cstring>
@@ -25,8 +27,12 @@ namespace test {
 
 void
 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*>(0), segment.getNamedAddress("test address"));
+    EXPECT_EQ(static_cast<void*>(NULL),
+              segment.getNamedAddress("test address"));
 
     // Now set it
     void* ptr32 = segment.allocate(sizeof(uint32_t));
@@ -34,6 +40,9 @@ checkSegmentNamedAddress(MemorySegment& segment, bool out_of_segment_ok) {
     std::memcpy(ptr32, &test_val, sizeof(test_val));
     EXPECT_FALSE(segment.setNamedAddress("test address", ptr32));
 
+    // NULL name isn't allowed.
+    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(test_val, *static_cast<const uint32_t*>(ptr32));
@@ -48,14 +57,16 @@ checkSegmentNamedAddress(MemorySegment& segment, bool out_of_segment_ok) {
 
     // Clear it.  Then we won't be able to find it any more.
     EXPECT_TRUE(segment.clearNamedAddress("test address"));
-    EXPECT_EQ(static_cast<void*>(0), segment.getNamedAddress("test address"));
+    EXPECT_EQ(static_cast<void*>(NULL),
+              segment.getNamedAddress("test address"));
 
     // 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", 0));
-    EXPECT_EQ(static_cast<void*>(0), segment.getNamedAddress("null address"));
+    EXPECT_EQ(static_cast<void*>(NULL),
+              segment.getNamedAddress("null address"));
 
     // If the underlying implementation performs explicit check against
     // out-of-segment address, confirm the behavior.
@@ -71,6 +82,8 @@ checkSegmentNamedAddress(MemorySegment& segment, bool out_of_segment_ok) {
     segment.deallocate(ptr16, sizeof(uint16_t));  // not yet
     EXPECT_FALSE(segment.allMemoryDeallocated());
     EXPECT_TRUE(segment.clearNamedAddress("null address"));
+    // null name isn't allowed:
+    EXPECT_THROW(segment.clearNamedAddress(NULL), InvalidParameter);
     EXPECT_TRUE(segment.allMemoryDeallocated()); // now everything is gone
 }