Utility: add better Directory::map(), rename original to mapWrite().

The original one doesn't preserve file contents, so it's practically write-only. Its original intended purpose was a "swap file" on the disk and should have been named mapWrite() right from the start. The new map() is like mapRead() but with read-write access and failing when a file doesn't exist. The original map() signature is still present, but marked as deprecated.
mosra · Jan 6, 2020 · 37f0682 · 37f0682
1 parent 1827e5b
commit 37f0682
Show file tree

Hide file tree

Showing 4 changed files with 213 additions and 55 deletions.
diff --git a/doc/corrade-changelog.dox b/doc/corrade-changelog.dox
@@ -77,6 +77,8 @@ namespace Corrade {
 -   Added a @ref CORRADE_STD_IS_TRIVIALLY_TRAITS_SUPPORTED macro into
     @ref Corrade/Utility/TypeTraits.h denoting if @ref std::is_trivially_copyable
     is available in the standard library
+-   Added @ref Utility::Directory::map(const std::string&) that provides
+    read-write access to mapped files without truncating them.
 
 @subsection corrade-changelog-latest-changes Changes and improvements
 
@@ -123,6 +125,13 @@ namespace Corrade {
 
 -   Various fixes (see [mosra/corrade#80](https://github.com/mosra/corrade/pull/80))
 
+@subsection corrade-changelog-latest-deprecated Deprecated APIs
+
+-   @cpp Utility::Directory::map(const std::string&, std::size_t) @ce is
+    deprecated and renamed to @ref Utility::Directory::mapWrite() as it doesn't
+    preserve original file contents and thus can't be used for read-write
+    access
+
 @subsection corrade-changelog-latest-compatibility Potential compatibility breakages, removed APIs
 
 -   @ref Containers::Array and @ref Containers::StaticArray was switched to

diff --git a/src/Corrade/Utility/Directory.cpp b/src/Corrade/Utility/Directory.cpp
@@ -759,34 +759,24 @@ void MapDeleter::operator()(const char* const data, const std::size_t size) {
     if(_fd) close(_fd);
 }
 
-Containers::Array<char, MapDeleter> map(const std::string& filename, std::size_t size) {
-    /* Open the file for writing. Create if it doesn't exist, truncate it if it
-       does. */
-    const int fd = open(filename.data(), O_RDWR|O_CREAT|O_TRUNC, mode_t(0600));
+Containers::Array<char, MapDeleter> map(const std::string& filename) {
+    /* Open the file for reading */
+    const int fd = open(filename.data(), O_RDWR);
     if(fd == -1) {
-        Error() << "Utility::Directory::map(): can't open" << filename;
-        return nullptr;
-    }
-
-    /* Resize the file to requested size by seeking one byte before */
-    if(lseek(fd, size - 1, SEEK_SET) == -1) {
-        close(fd);
-        Error() << "Utility::Directory::map(): can't seek to resize the file";
+        Error{} << "Utility::Directory::map(): can't open" << filename;
         return nullptr;
     }
 
-    /* And then writing a zero byte on that position */
-    if(::write(fd, "", 1) != 1) {
-        close(fd);
-        Error() << "Utility::Directory::map(): can't write to resize the file";
-        return nullptr;
-    }
+    /* Get file size */
+    const off_t currentPos = lseek(fd, 0, SEEK_CUR);
+    const std::size_t size = lseek(fd, 0, SEEK_END);
+    lseek(fd, currentPos, SEEK_SET);
 
     /* Map the file */
     char* data = reinterpret_cast<char*>(mmap(nullptr, size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0));
     if(data == MAP_FAILED) {
         close(fd);
-        Error() << "Utility::Directory::map(): can't map the file";
+        Error{} << "Utility::Directory::map(): can't map the file";
         return nullptr;
     }
 
@@ -816,31 +806,68 @@ Containers::Array<const char, MapDeleter> mapRead(const std::string& filename) {
 
     return Containers::Array<const char, MapDeleter>{data, size, MapDeleter{fd}};
 }
+
+Containers::Array<char, MapDeleter> mapWrite(const std::string& filename, std::size_t size) {
+    /* Open the file for writing. Create if it doesn't exist, truncate it if it
+       does. */
+    const int fd = open(filename.data(), O_RDWR|O_CREAT|O_TRUNC, mode_t(0600));
+    if(fd == -1) {
+        Error{} << "Utility::Directory::mapWrite(): can't open" << filename;
+        return nullptr;
+    }
+
+    /* Resize the file to requested size by seeking one byte before */
+    if(lseek(fd, size - 1, SEEK_SET) == -1) {
+        close(fd);
+        Error{} << "Utility::Directory::mapWrite(): can't seek to resize the file";
+        return nullptr;
+    }
+
+    /* And then writing a zero byte on that position */
+    if(::write(fd, "", 1) != 1) {
+        close(fd);
+        Error{} << "Utility::Directory::mapWrite(): can't write to resize the file";
+        return nullptr;
+    }
+
+    /* Map the file */
+    char* data = reinterpret_cast<char*>(mmap(nullptr, size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0));
+    if(data == MAP_FAILED) {
+        close(fd);
+        Error{} << "Utility::Directory::mapWrite(): can't map the file";
+        return nullptr;
+    }
+
+    return Containers::Array<char, MapDeleter>{data, size, MapDeleter{fd}};
+}
 #elif defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT)
 void MapDeleter::operator()(const char* const data, const std::size_t) {
     if(data) UnmapViewOfFile(data);
     if(_hMap) CloseHandle(_hMap);
     if(_hFile) CloseHandle(_hFile);
 }
 
-Containers::Array<char, MapDeleter> map(const std::string& filename, std::size_t size) {
+Containers::Array<char, MapDeleter> map(const std::string& filename) {
     /* Open the file for writing. Create if it doesn't exist, truncate it if it
        does. */
     HANDLE hFile = CreateFileW(widen(filename).data(),
-        GENERIC_READ|GENERIC_WRITE, FILE_SHARE_READ|FILE_SHARE_WRITE, nullptr, CREATE_ALWAYS, 0, nullptr);
+        GENERIC_READ|GENERIC_WRITE, FILE_SHARE_READ|FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, 0, nullptr);
     if (hFile == INVALID_HANDLE_VALUE) {
         Error() << "Utility::Directory::map(): can't open" << filename;
         return nullptr;
     }
 
     /* Create the file mapping */
-    HANDLE hMap = CreateFileMappingW(hFile, nullptr, PAGE_READWRITE, 0, size, nullptr);
+    HANDLE hMap = CreateFileMappingW(hFile, nullptr, PAGE_READWRITE, 0, 0, nullptr);
     if (!hMap) {
         Error() << "Utility::Directory::map(): can't create the file mapping:" << GetLastError();
         CloseHandle(hFile);
         return nullptr;
     }
 
+    /* Get file size */
+    const size_t size = GetFileSize(hFile, nullptr);
+
     /* Map the file */
     char* data = reinterpret_cast<char*>(::MapViewOfFile(hMap, FILE_MAP_ALL_ACCESS, 0, 0, 0));
     if(!data) {
@@ -884,6 +911,42 @@ Containers::Array<const char, MapDeleter> mapRead(const std::string& filename) {
 
     return Containers::Array<const char, MapDeleter>{data, size, MapDeleter{hFile, hMap}};
 }
+
+Containers::Array<char, MapDeleter> mapWrite(const std::string& filename, std::size_t size) {
+    /* Open the file for writing. Create if it doesn't exist, truncate it if it
+       does. */
+    HANDLE hFile = CreateFileW(widen(filename).data(),
+        GENERIC_READ|GENERIC_WRITE, FILE_SHARE_READ|FILE_SHARE_WRITE, nullptr, CREATE_ALWAYS, 0, nullptr);
+    if (hFile == INVALID_HANDLE_VALUE) {
+        Error() << "Utility::Directory::mapWrite(): can't open" << filename;
+        return nullptr;
+    }
+
+    /* Create the file mapping */
+    HANDLE hMap = CreateFileMappingW(hFile, nullptr, PAGE_READWRITE, 0, size, nullptr);
+    if (!hMap) {
+        Error() << "Utility::Directory::mapWrite(): can't create the file mapping:" << GetLastError();
+        CloseHandle(hFile);
+        return nullptr;
+    }
+
+    /* Map the file */
+    char* data = reinterpret_cast<char*>(::MapViewOfFile(hMap, FILE_MAP_ALL_ACCESS, 0, 0, 0));
+    if(!data) {
+        Error() << "Utility::Directory::mapWrite(): can't map the file:" << GetLastError();
+        CloseHandle(hMap);
+        CloseHandle(hFile);
+        return nullptr;
+    }
+
+    return Containers::Array<char, MapDeleter>{data, size, MapDeleter{hFile, hMap}};
+}
+#endif
+
+#if defined(CORRADE_BUILD_DEPRECATED) && (defined(CORRADE_TARGET_UNIX) || (defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT)))
+Containers::Array<char, MapDeleter> map(const std::string& filename, const std::size_t size) {
+    return mapWrite(filename, size);
+}
 #endif
 
 }}}
diff --git a/src/Corrade/Utility/Directory.h b/src/Corrade/Utility/Directory.h
@@ -483,17 +483,17 @@ CORRADE_UTILITY_EXPORT bool copy(const std::string& from, const std::string& to)
 #if defined(DOXYGEN_GENERATING_OUTPUT) || defined(CORRADE_TARGET_UNIX) || (defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT))
 /**
 @brief Map file for reading and writing
+@m_since_latest
 
-Maps the file as read-write memory and enlarges it to @p size. If the file does
-not exist yet, it is created, if it exists, it's truncated. The array deleter
-takes care of unmapping, however the file is not deleted after unmapping. If an
-error occurs, @cpp nullptr @ce is returned and a message is printed to
-@ref Error. Expects that the filename is in UTF-8.
-@see @ref mapRead(), @ref read(), @ref write()
+Maps the file as read-write memory. The array deleter takes care of unmapping.
+If the file doesn't exist or an error occurs while mapping, @cpp nullptr @ce is
+returned and a message is printed to @ref Error. Expects that the filename is
+in UTF-8.
+@see @ref mapRead(), @ref mapWrite(), @ref read(), @ref write()
 @partialsupport Available only on @ref CORRADE_TARGET_UNIX "Unix" and non-RT
     @ref CORRADE_TARGET_WINDOWS "Windows" platforms.
-    */
-CORRADE_UTILITY_EXPORT Containers::Array<char, MapDeleter> map(const std::string& filename, std::size_t size);
+*/
+CORRADE_UTILITY_EXPORT Containers::Array<char, MapDeleter> map(const std::string& filename);
 
 /**
 @brief Map file for reading
@@ -502,11 +502,34 @@ Maps the file as read-only memory. The array deleter takes care of unmapping.
 If the file doesn't exist or an error occurs while mapping, @cpp nullptr @ce is
 returned and a message is printed to @ref Error. Expects that the filename is
 in UTF-8.
-@see @ref map(), @ref read()
+@see @ref map(), @ref mapWrite(), @ref read()
 @partialsupport Available only on @ref CORRADE_TARGET_UNIX "Unix" and non-RT
     @ref CORRADE_TARGET_WINDOWS "Windows" platforms.
 */
 CORRADE_UTILITY_EXPORT Containers::Array<const char, MapDeleter> mapRead(const std::string& filename);
+
+/**
+@brief Map file for writing
+@m_since_latest
+
+Maps the file as read-write memory and enlarges it to @p size. If the file does
+not exist yet, it is created, if it exists, it's truncated --- thus no data
+is preserved. The array deleter takes care of unmapping, however the file is
+not deleted after unmapping. If an error occurs, @cpp nullptr @ce is returned
+and a message is printed to @ref Error. Expects that the filename is in UTF-8.
+@see @ref map(), @ref mapRead(), @ref read(), @ref write()
+@partialsupport Available only on @ref CORRADE_TARGET_UNIX "Unix" and non-RT
+    @ref CORRADE_TARGET_WINDOWS "Windows" platforms.
+*/
+CORRADE_UTILITY_EXPORT Containers::Array<char, MapDeleter> mapWrite(const std::string& filename, std::size_t size);
+
+#ifdef CORRADE_BUILD_DEPRECATED
+/**
+ * @copybrief mapWrite()
+ * @m_deprecated_since_latest Use @ref mapWrite() instead.
+ */
+CORRADE_DEPRECATED("use mapWrite() instead") CORRADE_UTILITY_EXPORT Containers::Array<char, MapDeleter> map(const std::string& filename, std::size_t size);
+#endif
 #endif
 
 #ifndef DOXYGEN_GENERATING_OUTPUT