Merge pull request #855 from capnproto/empty-mmap

Fix filesystem API: Empty mmaps should succeed.

c++/src/kj/filesystem-disk-test.c++

@@ -276,6 +276,11 @@ KJ_TEST("DiskFile") {
 
   KJ_EXPECT(file->readAllText() == "");
 
+  // mmaping empty file should work
+  KJ_EXPECT(file->mmap(0, 0).size() == 0);
+  KJ_EXPECT(file->mmapPrivate(0, 0).size() == 0);
+  KJ_EXPECT(file->mmapWritable(0, 0)->get().size() == 0);
+
   file->writeAll("foo");
   KJ_EXPECT(file->readAllText() == "foo");
 
@@ -294,6 +299,14 @@ KJ_TEST("DiskFile") {
   file->truncate(18);
   KJ_EXPECT(file->readAllText() == kj::StringPtr("foobaz\0\0\0\0\0\0\0\0\0\0\0\0", 18));
 
+  // empty mappings work, even if useless
+  KJ_EXPECT(file->mmap(0, 0).size() == 0);
+  KJ_EXPECT(file->mmapPrivate(0, 0).size() == 0);
+  KJ_EXPECT(file->mmapWritable(0, 0)->get().size() == 0);
+  KJ_EXPECT(file->mmap(2, 0).size() == 0);
+  KJ_EXPECT(file->mmapPrivate(2, 0).size() == 0);
+  KJ_EXPECT(file->mmapWritable(2, 0)->get().size() == 0);
+
   {
     auto mapping = file->mmap(0, 18);
     auto privateMapping = file->mmapPrivate(0, 18);

c++/src/kj/filesystem-disk-unix.c++

@@ -346,6 +346,7 @@ public:
   }
 
   Array<const byte> mmap(uint64_t offset, uint64_t size) const {
+    if (size == 0) return nullptr;  // zero-length mmap() returns EINVAL, so avoid it
     auto range = getMmapRange(offset, size);
     const void* mapping = ::mmap(NULL, range.size, PROT_READ, MAP_SHARED, fd, range.offset);
     if (mapping == MAP_FAILED) {
@@ -356,6 +357,7 @@ public:
   }
 
   Array<byte> mmapPrivate(uint64_t offset, uint64_t size) const {
+    if (size == 0) return nullptr;  // zero-length mmap() returns EINVAL, so avoid it
     auto range = getMmapRange(offset, size);
     void* mapping = ::mmap(NULL, range.size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, range.offset);
     if (mapping == MAP_FAILED) {
@@ -454,6 +456,7 @@ public:
     void changed(ArrayPtr<byte> slice) const override {
       KJ_REQUIRE(slice.begin() >= bytes.begin() && slice.end() <= bytes.end(),
                  "byte range is not part of this mapping");
+      if (slice.size() == 0) return;
 
       // msync() requires page-alignment, apparently, so use getMmapRange() to accomplish that.
       auto range = getMmapRange(reinterpret_cast<uintptr_t>(slice.begin()), slice.size());
@@ -463,6 +466,7 @@ public:
     void sync(ArrayPtr<byte> slice) const override {
       KJ_REQUIRE(slice.begin() >= bytes.begin() && slice.end() <= bytes.end(),
                  "byte range is not part of this mapping");
+      if (slice.size() == 0) return;
 
       // msync() requires page-alignment, apparently, so use getMmapRange() to accomplish that.
       auto range = getMmapRange(reinterpret_cast<uintptr_t>(slice.begin()), slice.size());
@@ -474,6 +478,10 @@ public:
   };
 
   Own<const WritableFileMapping> mmapWritable(uint64_t offset, uint64_t size) const {
+    if (size == 0) {
+      // zero-length mmap() returns EINVAL, so avoid it
+      return heap<WritableFileMappingImpl>(nullptr);
+    }
     auto range = getMmapRange(offset, size);
     void* mapping = ::mmap(NULL, range.size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, range.offset);
     if (mapping == MAP_FAILED) {

c++/src/kj/filesystem-disk-win32.c++

@@ -307,16 +307,7 @@ constexpr MmapDisposer mmapDisposer = MmapDisposer();
 
 void* win32Mmap(HANDLE handle, MmapRange range, DWORD pageProtect, DWORD access) {
   HANDLE mappingHandle;
-  mappingHandle = CreateFileMappingW(handle, NULL, pageProtect, 0, 0, NULL);
-  if (mappingHandle == INVALID_HANDLE_VALUE) {
-    auto error = GetLastError();
-    if (error == ERROR_FILE_INVALID && range.size == 0) {
-      // The documentation says that CreateFileMapping will fail with ERROR_FILE_INVALID if the
-      // file size is zero. Ugh.
-      return nullptr;
-    }
-    KJ_FAIL_WIN32("CreateFileMapping", error);
-  }
+  KJ_WIN32(mappingHandle = CreateFileMappingW(handle, NULL, pageProtect, 0, 0, NULL));
   KJ_DEFER(KJ_WIN32(CloseHandle(mappingHandle)) { break; });
 
   void* mapping = MapViewOfFile(mappingHandle, access,
@@ -428,6 +419,7 @@ public:
   }
 
   Array<const byte> mmap(uint64_t offset, uint64_t size) const {
+    if (size == 0) return nullptr;  // Windows won't allow zero-length mappings
     auto range = getMmapRange(offset, size);
     const void* mapping = win32Mmap(handle, range, PAGE_READONLY, FILE_MAP_READ);
     return Array<const byte>(reinterpret_cast<const byte*>(mapping) + (offset - range.offset),
@@ -435,6 +427,7 @@ public:
   }
 
   Array<byte> mmapPrivate(uint64_t offset, uint64_t size) const {
+    if (size == 0) return nullptr;  // Windows won't allow zero-length mappings
     auto range = getMmapRange(offset, size);
     void* mapping = win32Mmap(handle, range, PAGE_READONLY, FILE_MAP_COPY);
     return Array<byte>(reinterpret_cast<byte*>(mapping) + (offset - range.offset),
@@ -552,7 +545,8 @@ public:
       KJ_REQUIRE(slice.begin() >= bytes.begin() && slice.end() <= bytes.end(),
                  "byte range is not part of this mapping");
 
-      // Zero is treated specially by FlushViewOfFile(), so check for it.
+      // Zero is treated specially by FlushViewOfFile(), so check for it. (This also handles the
+      // case where `bytes` is actually empty and not a real mapping.)
       if (slice.size() > 0) {
         KJ_WIN32(FlushViewOfFile(slice.begin(), slice.size()));
       }
@@ -563,6 +557,10 @@ public:
   };
 
   Own<const WritableFileMapping> mmapWritable(uint64_t offset, uint64_t size) const {
+    if (size == 0) {
+      // Windows won't allow zero-length mappings
+      return heap<WritableFileMappingImpl>(nullptr);
+    }
     auto range = getMmapRange(offset, size);
     void* mapping = win32Mmap(handle, range, PAGE_READWRITE, FILE_MAP_ALL_ACCESS);
     auto array = Array<byte>(reinterpret_cast<byte*>(mapping) + (offset - range.offset),