[2831] updated rw ctor so that we can create a new segment, removing old one.

also clarified that in other read-write open existing file isn't modified.

src/lib/util/memory_segment_mapped.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <util/memory_segment_mapped.h>
+#include <util/unittests/check_valgrind.h>
 
 #include <exceptions/exceptions.h>
 
@@ -46,12 +47,21 @@ typedef basic_managed_mapped_file<char,
                                   iset_index> BaseSegment;
 
 struct MemorySegmentMapped::Impl {
-    Impl(const std::string& filename, size_t initial_size) :
+    // Constructor for create-only (and read-write) mode
+    Impl(const std::string& filename, create_only_t, size_t initial_size) :
+        read_only_(false), filename_(filename),
+        base_sgmt_(new BaseSegment(create_only, filename.c_str(),
+                                   initial_size))
+    {}
+
+    // Constructor for open-or-write (and read-write) mode
+    Impl(const std::string& filename, open_or_create_t, size_t initial_size) :
         read_only_(false), filename_(filename),
         base_sgmt_(new BaseSegment(open_or_create, filename.c_str(),
                                    initial_size))
     {}
 
+    // Constructor for existing segment, either read-only or read-write
     Impl(const std::string& filename, bool read_only) :
         read_only_(read_only), filename_(filename),
         base_sgmt_(read_only ?
@@ -110,14 +120,25 @@ MemorySegmentMapped::MemorySegmentMapped(const std::string& filename) :
 }
 
 MemorySegmentMapped::MemorySegmentMapped(const std::string& filename,
-                                         bool create, size_t initial_size) :
+                                         OpenMode mode, size_t initial_size) :
     impl_(NULL)
 {
     try {
-        if (create) {
-            impl_ = new Impl(filename, initial_size);
-        } else {
+        switch (mode) {
+        case OPEN_FOR_WRITE:
             impl_ = new Impl(filename, false);
+            break;
+        case OPEN_OR_CREATE:
+            impl_ = new Impl(filename, open_or_create, initial_size);
+            break;
+        case CREATE_ONLY:
+            // Remove any existing file then create a new one
+            file_mapping::remove(filename.c_str());
+            impl_ = new Impl(filename, create_only, initial_size);
+            break;
+        default:
+            isc_throw(InvalidParameter,
+                      "invalid open mode for MemorySegmentMapped: " << mode);
         }
     } catch (const boost::interprocess::interprocess_exception& ex) {
         isc_throw(MemorySegmentOpenError,

src/lib/util/memory_segment_mapped.h

@@ -50,6 +50,16 @@ public:
     /// sufficiently but not too large.
     static const size_t INITIAL_SIZE = 32768;
 
+    /// \brief Open modes of \c MemorySegmentMapped.
+    ///
+    /// These modes matter only for \c MemorySegmentMapped to be opened
+    /// in read-write mode, and specify further details of open operation.
+    enum OpenMode {
+        OPEN_FOR_WRITE = 0,     ///< Open only.  File must exist.
+        OPEN_OR_CREATE,         ///< If file doesn't exist it's created.
+        CREATE_ONLY ///< New file is created; existing one will be removed.
+    };
+
     /// \brief Constructor in the read-only mode.
     ///
     /// This constructor will map the content of the given file into memory
@@ -75,13 +85,25 @@ public:
     ///
     /// This is similar to the read-only version of the constructor, but
     /// does not have the restrictions that the read-only version has.
-    /// If \c create is true and the specified file does not exist, this
-    /// method tries to create a new file of the name and build internal
-    /// data on it so that the file will be mappable by this class
-    /// object.  If \c create is false, the specified file must exist
+    ///
+    /// The \c mode parameter specifies further details of how the segment
+    /// should be opened.
+    /// - OPEN_FOR_WRITE: this is open-only mode.  The file must exist,
+    ///   and it will be opened without any initial modification.
+    /// - OPEN_OR_CREATE: similar to OPEN_FOR_WRITE, but if the file does not
+    ///   exist, a new one will be created.  An existing file will be used
+    ///   any initial modification.
+    /// - CREATE_ONLY: a new file (of the given file name) will be created;
+    ///   any existing file of the same name will be removed.
+    ///
+    /// If OPEN_FOR_WRITE is specified, the specified file must exist
     /// and be writable, and have been previously initialized by this
-    /// version of constructor with \c create being true.  If any of
-    /// these conditions is not met, \c MemorySegmentOpenError exception
+    /// version of constructor either with OPEN_OR_CREATE or CREATE_ONLY.
+    /// If the mode is OPEN_OR_CREATE or CREATE_ONLY, and the file needs
+    /// to be created, then this method tries to create a new file of the
+    /// name and build internal data on it so that the file will be mappable
+    /// by this class object.  If any of these conditions is not met, or
+    /// create or initialization fails, \c MemorySegmentOpenError exception
     /// will be thrown.
     ///
     /// When initial_size is specified but is too small (including a value of
@@ -93,11 +115,10 @@ public:
     /// \throw MemorySegmentOpenError see the description.
     ///
     /// \param filename The file name to be mapped to memory.
-    /// \param create If true and the file does not exist, a new one is
-    /// created.
+    /// \param mode Open mode (see the description).
     /// \param initial_size Specifies the size of the newly created file;
-    /// ignored if \c create is false.
-    MemorySegmentMapped(const std::string& filename, bool create,
+    /// ignored if \c mode is OPEN_FOR_WRITE.
+    MemorySegmentMapped(const std::string& filename, OpenMode mode,
                         size_t initial_size = INITIAL_SIZE);
 
     /// \brief Destructor.

src/lib/util/tests/memory_segment_mapped_unittest.cc

@@ -44,6 +44,13 @@ using boost::scoped_ptr;
 using isc::util::test::parentReadState;
 
 namespace {
+// Shortcut to keep code shorter
+const MemorySegmentMapped::OpenMode OPEN_FOR_WRITE =
+    MemorySegmentMapped::OPEN_FOR_WRITE;
+const MemorySegmentMapped::OpenMode OPEN_OR_CREATE =
+    MemorySegmentMapped::OPEN_OR_CREATE;
+const MemorySegmentMapped::OpenMode CREATE_ONLY =
+    MemorySegmentMapped::CREATE_ONLY;
 
 const char* const mapped_file = TEST_DATA_BUILDDIR "/test.mapped";
 const size_t DEFAULT_INITIAL_SIZE = 32 * 1024; // intentionally hardcoded
@@ -64,7 +71,7 @@ protected:
     void resetSegment() {
         segment_.reset();
         boost::interprocess::file_mapping::remove(mapped_file);
-        segment_.reset(new MemorySegmentMapped(mapped_file, true));
+        segment_.reset(new MemorySegmentMapped(mapped_file, OPEN_OR_CREATE));
     }
 
     scoped_ptr<MemorySegmentMapped> segment_;
@@ -94,7 +101,7 @@ TEST_F(MemorySegmentMappedTest, createAndModify) {
 
         // re-open it in read-write mode, but don't try to create it
         // this time.
-        segment_.reset(new MemorySegmentMapped(mapped_file, false));
+        segment_.reset(new MemorySegmentMapped(mapped_file, OPEN_FOR_WRITE));
     }
 }
 
@@ -105,18 +112,31 @@ TEST_F(MemorySegmentMappedTest, createWithSize) {
     // the size is actually the specified one.
     const size_t new_size = 64 * 1024;
     EXPECT_NE(new_size, segment_->getSize());
-    segment_.reset(new MemorySegmentMapped(mapped_file, true, new_size));
+    segment_.reset(new MemorySegmentMapped(mapped_file, OPEN_OR_CREATE,
+                                           new_size));
     EXPECT_EQ(new_size, segment_->getSize());
 }
 
+TEST_F(MemorySegmentMappedTest, createOnly) {
+    // First, allocate some data in the existing segment
+    EXPECT_TRUE(segment_->allocate(16));
+    // Close it, and then open it again in the create-only mode.  the existing
+    // file should be internally removed, and so the resulting segment
+    // should be "empty" (all deallocated).
+    segment_.reset();
+    segment_.reset(new MemorySegmentMapped(mapped_file, CREATE_ONLY));
+    EXPECT_TRUE(segment_->allMemoryDeallocated());
+}
+
 TEST_F(MemorySegmentMappedTest, openFail) {
     // The given file is directory
-    EXPECT_THROW(MemorySegmentMapped("/", true), MemorySegmentOpenError);
+    EXPECT_THROW(MemorySegmentMapped("/", OPEN_OR_CREATE),
+                 MemorySegmentOpenError);
 
     // file doesn't exist and directory isn't writable (we assume the
     // following path is not writable for the user running the test).
-    EXPECT_THROW(MemorySegmentMapped("/random-glkwjer098/test.mapped", true),
-                 MemorySegmentOpenError);
+    EXPECT_THROW(MemorySegmentMapped("/random-glkwjer098/test.mapped",
+                                     OPEN_OR_CREATE), MemorySegmentOpenError);
 
     // It should fail when file doesn't exist and it's read-only (so
     // open-only).
@@ -125,15 +145,21 @@ TEST_F(MemorySegmentMappedTest, openFail) {
     // Likewise, it should fail in read-write mode when creation is
     // suppressed.
     EXPECT_THROW(MemorySegmentMapped(TEST_DATA_BUILDDIR "/nosuchfile.mapped",
-                                     false),
-                 MemorySegmentOpenError);
+                                     OPEN_FOR_WRITE), MemorySegmentOpenError);
 
     // creating with a very small size fails (for sure about 0, and other
     // small values should also make it fail, but it's internal restriction
     // of Boost and cannot be predictable).
-    EXPECT_THROW(MemorySegmentMapped(mapped_file, true, 0),
+    EXPECT_THROW(MemorySegmentMapped(mapped_file, OPEN_OR_CREATE, 0),
                  MemorySegmentOpenError);
 
+    // invalid read-write mode
+    EXPECT_THROW(MemorySegmentMapped(
+                     mapped_file,
+                     static_cast<MemorySegmentMapped::OpenMode>(
+                         static_cast<int>(CREATE_ONLY) + 1)),
+                 isc::InvalidParameter);
+
     // Close the existing segment, break its file with bogus data, and
     // try to reopen.  It should fail with exception whether in the
     // read-only or read-write, or "create if not exist" mode.
@@ -142,9 +168,9 @@ TEST_F(MemorySegmentMappedTest, openFail) {
     ofs << std::string(1024, 'x');
     ofs.close();
     EXPECT_THROW(MemorySegmentMapped sgmt(mapped_file), MemorySegmentOpenError);
-    EXPECT_THROW(MemorySegmentMapped sgmt(mapped_file, false),
+    EXPECT_THROW(MemorySegmentMapped sgmt(mapped_file, OPEN_FOR_WRITE),
                  MemorySegmentOpenError);
-    EXPECT_THROW(MemorySegmentMapped sgmt(mapped_file, true),
+    EXPECT_THROW(MemorySegmentMapped sgmt(mapped_file, OPEN_OR_CREATE),
                  MemorySegmentOpenError);
 }
 
@@ -277,7 +303,7 @@ TEST_F(MemorySegmentMappedTest, namedAddress) {
     // segment extension.
     segment_.reset();
     boost::interprocess::file_mapping::remove(mapped_file);
-    segment_.reset(new MemorySegmentMapped(mapped_file, true, 1024));
+    segment_.reset(new MemorySegmentMapped(mapped_file, OPEN_OR_CREATE, 1024));
     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));
@@ -288,7 +314,7 @@ TEST_F(MemorySegmentMappedTest, namedAddress) {
     // shrinking segment.
     segment_.reset();
     boost::interprocess::file_mapping::remove(mapped_file);
-    segment_.reset(new MemorySegmentMapped(mapped_file, true));
+    segment_.reset(new MemorySegmentMapped(mapped_file, OPEN_OR_CREATE));
     std::map<std::string, std::vector<uint8_t> > data_list;
     data_list["data1"] =
         std::vector<uint8_t>(80); // arbitrarily chosen small data