[2831] make shrinkToFit non virtual; add more doc

src/lib/util/memory_segment.h

@@ -174,8 +174,6 @@ public:
     /// \param name A C string of which the segment memory address is to be
     /// deleted.
     virtual bool clearNamedAddress(const char* name) = 0;
-
-    virtual void shrinkToFit() = 0;
 };
 
 } // namespace util

src/lib/util/memory_segment_local.h

@@ -70,9 +70,6 @@ public:
     virtual bool setNamedAddress(const char* name, void* addr);
     virtual bool clearNamedAddress(const char* name);
 
-    /// There's nothing this implementation can do for this.
-    virtual void shrinkToFit() {}
-
 private:
     // allocated_size_ can underflow, wrap around to max size_t (which
     // is unsigned). But because we only do a check against 0 and not a

src/lib/util/memory_segment_mapped.cc

@@ -143,6 +143,10 @@ MemorySegmentMapped::allocate(size_t size) {
 
 void
 MemorySegmentMapped::deallocate(void* ptr, size_t) {
+    if (impl_->read_only_) {
+        isc_throw(InvalidOperation, "allocate attempt on read-only segment");
+    }
+
     // the underlying deallocate() would deal with the case where ptr == 0,
     // but it's an undocumented behavior, so we handle it ourselves for safety.
     if (!ptr) {

src/lib/util/memory_segment_mapped.h

@@ -43,6 +43,11 @@ public:
     virtual ~MemorySegmentMapped();
 
     /// \brief Allocate/acquire a segment of memory.
+    ///
+    /// This version can throw \c MemorySegmentGrown.
+    ///
+    /// This method cannot be called if the segment object is created in the
+    /// read-only mode; in that case InvalidOperation will be thrown.
     virtual void* allocate(size_t size);
 
     /// \brief Deallocate/release a segment of memory.
@@ -59,22 +64,61 @@ public:
     /// since then; if it was thrown the corresponding address must have been
     /// adjusted some way; e.g., by re-fetching the latest mapped address
     /// via \c getNamedAddress().
+    ///
+    /// This method cannot be called if the segment object is created in the
+    /// read-only mode; in that case InvalidOperation will be thrown.
     virtual void deallocate(void* ptr, size_t size);
 
     virtual bool allMemoryDeallocated() const;
 
-    virtual void* getNamedAddress(const char* name);
-
-    /// \brief TBD
+    /// \brief Mapped segment version of setNamedAddress.
     ///
     /// This implementation detects if \c addr is invalid (see the base class
     /// description) and throws \c MemorySegmentError in that case.
+    ///
+    /// This version can actually return true.
+    ///
+    /// 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);
 
+    /// \brief Mapped segment version of getNamedAddress.
+    ///
+    /// This version never throws.
+    virtual void* getNamedAddress(const char* name);
+
+    /// \brief Mapped segment version of clearNamedAddress.
+    ///
+    /// This version never throws.
     virtual bool clearNamedAddress(const char* name);
 
-    virtual void shrinkToFit();
+    /// \brief Shrink the underlying mapped segment to actually used size.
+    ///
+    /// It will be convenient when a large amount of memory is allocated
+    /// then deallocated from the segment in order to keep the resulting
+    /// segment a reasonable size.
+    ///
+    /// This method works best effort basis, and does not guarantee any
+    /// specific result.
+    ///
+    /// This method is generally expected to be failure-free, but it's still
+    /// possible to fail.  For example, the underlying file may not be writable
+    /// at the time of shrink attempt; it also tries to remap the shrunk
+    /// segment internally, and there's a small chance it could fail.
+    /// In such a case it throws \c MemorySegmentError.  If it's thrown the
+    /// segment is not usable any more.
+    ///
+    /// This method cannot be called if the segment object is created in the
+    /// read-only mode; in that case InvalidOperation will be thrown.
+    void shrinkToFit();
 
+    /// \brief Return the actual segment size.
+    ///
+    /// This is generally expected to be the file size to map.  It's
+    /// provided mainly for diagnose an testing purposes; the application
+    /// shouldn't rely on specific return values of this method.
+    ///
+    /// \throw None
     size_t getSize() const;
 
 private:

src/lib/util/tests/memory_segment_mapped_unittest.cc

@@ -252,14 +252,18 @@ TEST_F(MemorySegmentMappedTest, violateReadOnly) {
     EXPECT_THROW(MemorySegmentMapped(mapped_file).shrinkToFit(),
                  isc::InvalidOperation);
 
+    void* ptr = segment_->allocate(sizeof(uint32_t));
+    segment_->setNamedAddress("test address", ptr);
+
     EXPECT_DEATH_IF_SUPPORTED({
-            void* ptr = segment_->allocate(sizeof(uint32_t));
-            segment_->setNamedAddress("test address", ptr);
             MemorySegmentMapped segment_ro(mapped_file);
             EXPECT_TRUE(segment_ro.getNamedAddress("test address"));
             *static_cast<uint32_t*>(
                 segment_ro.getNamedAddress("test address")) = 0;
         }, "");
+
+    EXPECT_THROW(MemorySegmentMapped(mapped_file).deallocate(ptr, 4),
+                 isc::InvalidOperation);
 }
 
 }