Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(417)

Unified Diff: base/files/memory_mapped_file_posix.cc

Issue 2860943005: Ensure that memory-maped files are fully realized. (Closed)
Patch Set: fix cross-platform build problems Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/files/memory_mapped_file_posix.cc
diff --git a/base/files/memory_mapped_file_posix.cc b/base/files/memory_mapped_file_posix.cc
index 90ba6f49c1573f6fb4fb1f56bc33f35b36febb06..59acc4a30cea5e76f261fbe859c82914e2abb520 100644
--- a/base/files/memory_mapped_file_posix.cc
+++ b/base/files/memory_mapped_file_posix.cc
@@ -4,6 +4,9 @@
#include "base/files/memory_mapped_file.h"
+#define _GNU_SOURCE 1 // Needed to get fallocate out of fcntl.h.
+
+#include <fcntl.h>
#include <stddef.h>
#include <stdint.h>
#include <sys/mman.h>
@@ -71,9 +74,11 @@ bool MemoryMappedFile::MapFileRegionToMemory(
case READ_ONLY:
flags |= PROT_READ;
break;
+
case READ_WRITE:
flags |= PROT_READ | PROT_WRITE;
break;
+
case READ_WRITE_EXTEND:
// POSIX won't auto-extend the file when it is written so it must first
// be explicitly extended to the maximum size. Zeros will fill the new
@@ -83,10 +88,39 @@ bool MemoryMappedFile::MapFileRegionToMemory(
DPLOG(ERROR) << "fstat " << file_.GetPlatformFile();
return false;
}
- file_.SetLength(std::max(file_len, region.offset + region.size));
flags |= PROT_READ | PROT_WRITE;
+
+ // Increase the actual length of the file, if necessary. This can fail if
+ // the disk is full and the OS doesn't support sparse files.
+ if (!file_.SetLength(std::max(file_len, region.offset + region.size))) {
Mark Mentovai 2017/05/04 21:00:47 This isn’t necessary if you’re going to call [posi
bcwhite 2017/05/05 18:06:59 Acknowledged.
+ DPLOG(ERROR) << "ftruncate " << file_.GetPlatformFile();
+ return false;
+ }
+
+// Realize the extent of the file so that it can't fail (and crash) later
Mark Mentovai 2017/05/04 21:00:47 This is one of clang-format’s problems, these comm
bcwhite 2017/05/05 18:06:59 Done.
+// when trying to write to a memory page that can't be created. This can
+// fail if the disk is full and the file is sparse.
+#if defined(OS_MACOSX)
+// Mac OSX doesn't support this call but the primary filesystem doesn't
+// support sparse files so is unneeded.
+#elif defined(OS_ANDROID)
+ // Android doesn't support the POSIX call so use the native Linux one.
Mark Mentovai 2017/05/04 21:00:46 Curious about this. It looks like both fallocate()
bcwhite 2017/05/05 15:48:00 Yeah, the Android builds all fail with either call
Primiano Tucci (use gerrit) 2017/05/05 16:23:33 FYI we currently ship 32 bit only chrome also on 6
bcwhite 2017/05/05 18:06:59 Acknowledged.
+ if (fallocate(file_.GetPlatformFile(), 0, region.offset, region.size) !=
Mark Mentovai 2017/05/04 21:00:46 Anyway, whether it’s fallocate() or posix_fallocat
bcwhite 2017/05/05 18:06:59 Done.
+ 0) {
+ DPLOG(ERROR) << "fallocate " << file_.GetPlatformFile();
+ return false;
+ }
+#else
+ if (posix_fallocate(file_.GetPlatformFile(), region.offset,
+ region.size) != 0) {
+ DPLOG(ERROR) << "posix_fallocate " << file_.GetPlatformFile();
+ return false;
+ }
+#endif
+
break;
}
+
data_ = static_cast<uint8_t*>(mmap(NULL, map_size, flags, MAP_SHARED,
file_.GetPlatformFile(), map_start));
if (data_ == MAP_FAILED) {
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698