Split getOpenFile into getOpenFile and getOpenFileSlice.

The main observation is that we never need both the filesize and the map size.
When mapping a slice of a file, it doesn't make sense to request a null
terminator and that would be the only case where the filesize would be used.

There are other cleanups that should be done in this area:

* A client should not have to pass the size (even an explicit -1) to say if
  it wants a null terminator or not, so we should probably swap the argument
  order.
* The default should be to not require a null terminator. Very few clients
  require this, but many end up asking for it just because it is the default.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@186984 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/Support/MemoryBuffer.h b/include/llvm/Support/MemoryBuffer.h
index e443b684633c4714774917d7eb73b5b626842add..9609e3366d4095fb5656c83d4cce2e176e4739ce 100644 (file)
--- a/include/llvm/Support/MemoryBuffer.h
+++ b/include/llvm/Support/MemoryBuffer.h
@@ -74,13 +74,17 @@ public:
                             int64_t FileSize = -1,
                             bool RequiresNullTerminator = true);
 
-  /// getOpenFile - Given an already-open file descriptor, read the file and
-  /// return a MemoryBuffer.
+  // Get a MemoryBuffer of part of a file. Since this is in the middle of a
+  // file, the buffer is not null terminated.
+  static error_code getOpenFileSlice(int FD, const char *Filename,
+                                     OwningPtr<MemoryBuffer> &Result,
+                                     uint64_t MapSize, int64_t Offset);
+
+  /// Given an already-open file descriptor, read the file and return a
+  /// MemoryBuffer.
   static error_code getOpenFile(int FD, const char *Filename,
-                                OwningPtr<MemoryBuffer> &result,
-                                uint64_t FileSize = -1,
-                                uint64_t MapSize = -1,
-                                int64_t Offset = 0,
+                                OwningPtr<MemoryBuffer> &Result,
+                                uint64_t FileSize,
                                 bool RequiresNullTerminator = true);
 
   /// getMemBuffer - Open the specified memory range as a MemoryBuffer.  Note

diff --git a/lib/Support/MemoryBuffer.cpp b/lib/Support/MemoryBuffer.cpp
index 051d64b0386c8d23c7b1f5bb1315503863ddbfa5..cab45c736b51e5fe585d208a87aea26fb3809272 100644 (file)
--- a/lib/Support/MemoryBuffer.cpp
+++ b/lib/Support/MemoryBuffer.cpp
@@ -248,6 +248,11 @@ error_code MemoryBuffer::getFile(StringRef Filename,
                                RequiresNullTerminator);
 }
 
+static error_code getOpenFileImpl(int FD, const char *Filename,
+                                  OwningPtr<MemoryBuffer> &Result,
+                                  uint64_t FileSize, uint64_t MapSize,
+                                  int64_t Offset, bool RequiresNullTerminator);
+
 error_code MemoryBuffer::getFile(const char *Filename,
                                  OwningPtr<MemoryBuffer> &result,
                                  int64_t FileSize,
@@ -257,8 +262,8 @@ error_code MemoryBuffer::getFile(const char *Filename,
   if (EC)
     return EC;
 
-  error_code ret = getOpenFile(FD, Filename, result, FileSize, FileSize,
-                               0, RequiresNullTerminator);
+  error_code ret = getOpenFileImpl(FD, Filename, result, FileSize, FileSize, 0,
+                                   RequiresNullTerminator);
   close(FD);
   return ret;
 }
@@ -305,11 +310,10 @@ static bool shouldUseMmap(int FD,
   return true;
 }
 
-error_code MemoryBuffer::getOpenFile(int FD, const char *Filename,
-                                     OwningPtr<MemoryBuffer> &result,
-                                     uint64_t FileSize, uint64_t MapSize,
-                                     int64_t Offset,
-                                     bool RequiresNullTerminator) {
+static error_code getOpenFileImpl(int FD, const char *Filename,
+                                  OwningPtr<MemoryBuffer> &result,
+                                  uint64_t FileSize, uint64_t MapSize,
+                                  int64_t Offset, bool RequiresNullTerminator) {
   static int PageSize = sys::process::get_self()->page_size();
 
   // Default is to map the full file.
@@ -386,6 +390,20 @@ error_code MemoryBuffer::getOpenFile(int FD, const char *Filename,
   return error_code::success();
 }
 
+error_code MemoryBuffer::getOpenFile(int FD, const char *Filename,
+                                     OwningPtr<MemoryBuffer> &Result,
+                                     uint64_t FileSize,
+                                     bool RequiresNullTerminator) {
+  return getOpenFileImpl(FD, Filename, Result, FileSize, FileSize, 0,
+                         RequiresNullTerminator);
+}
+
+error_code MemoryBuffer::getOpenFileSlice(int FD, const char *Filename,
+                                          OwningPtr<MemoryBuffer> &Result,
+                                          uint64_t MapSize, int64_t Offset) {
+  return getOpenFileImpl(FD, Filename, Result, -1, MapSize, Offset, false);
+}
+
 //===----------------------------------------------------------------------===//
 // MemoryBuffer::getSTDIN implementation.
 //===----------------------------------------------------------------------===//

diff --git a/tools/gold/gold-plugin.cpp b/tools/gold/gold-plugin.cpp
index 133eca48e50e070ef19eef4de097693e6856631e..77717098d61462701753209c71071e12cc889506 100644 (file)
--- a/tools/gold/gold-plugin.cpp
+++ b/tools/gold/gold-plugin.cpp
@@ -252,9 +252,8 @@ static ld_plugin_status claim_file_hook(const ld_plugin_input_file *file,
     if (file->offset) {
       offset = file->offset;
     }
-    if (error_code ec =
-        MemoryBuffer::getOpenFile(file->fd, file->name, buffer, -1,
-                                  file->filesize, offset, false)) {
+    if (error_code ec = MemoryBuffer::getOpenFileSlice(
+            file->fd, file->name, buffer, file->filesize, offset)) {
       (*message)(LDPL_ERROR, ec.message().c_str());
       return LDPS_ERR;
     }

diff --git a/tools/llvm-ar/llvm-ar.cpp b/tools/llvm-ar/llvm-ar.cpp
index 116143e1685c73c6abe8db865b6e4a063b188902..261446cc7826c868b85101e8de296949fd71b000 100644 (file)
--- a/tools/llvm-ar/llvm-ar.cpp
+++ b/tools/llvm-ar/llvm-ar.cpp
@@ -770,9 +770,9 @@ static void performWriteOperation(ArchiveOperation Operation,
       failIfError(sys::fs::status(FD, Status), FileName);
 
       OwningPtr<MemoryBuffer> File;
-      failIfError(
-          MemoryBuffer::getOpenFile(FD, FileName, File, Status.getSize()),
-          FileName);
+      failIfError(MemoryBuffer::getOpenFile(FD, FileName, File,
+                                            Status.getSize(), false),
+                  FileName);
 
       StringRef Name = sys::path::filename(FileName);
       if (Name.size() < 16)

diff --git a/tools/lto/LTOModule.cpp b/tools/lto/LTOModule.cpp
index 5ee43ba02292e0a72f627ccfb9ac5332d4cb5a7e..6626eaac47a8a7ab44d206cc1a0ecda04cb7bbf4 100644 (file)
--- a/tools/lto/LTOModule.cpp
+++ b/tools/lto/LTOModule.cpp
@@ -209,17 +209,16 @@ LTOModule *LTOModule::makeLTOModule(const char *path, std::string &errMsg) {
 
 LTOModule *LTOModule::makeLTOModule(int fd, const char *path,
                                     size_t size, std::string &errMsg) {
-  return makeLTOModule(fd, path, size, size, 0, errMsg);
+  return makeLTOModule(fd, path, size, 0, errMsg);
 }
 
 LTOModule *LTOModule::makeLTOModule(int fd, const char *path,
-                                    size_t file_size,
                                     size_t map_size,
                                     off_t offset,
                                     std::string &errMsg) {
   OwningPtr<MemoryBuffer> buffer;
-  if (error_code ec = MemoryBuffer::getOpenFile(fd, path, buffer, file_size,
-                                                map_size, offset, false)) {
+  if (error_code ec =
+          MemoryBuffer::getOpenFileSlice(fd, path, buffer, map_size, offset)) {
     errMsg = ec.message();
     return NULL;
   }

diff --git a/tools/lto/LTOModule.h b/tools/lto/LTOModule.h
index 83f3a7def1084c2e6d12f05b99837cd2c9cd7ddf..902e9c5438952dffa9003abcc989b578c1ac7e4a 100644 (file)
--- a/tools/lto/LTOModule.h
+++ b/tools/lto/LTOModule.h
@@ -82,7 +82,6 @@ public:
   static LTOModule *makeLTOModule(int fd, const char *path,
                                   size_t size, std::string &errMsg);
   static LTOModule *makeLTOModule(int fd, const char *path,
-                                  size_t file_size,
                                   size_t map_size,
                                   off_t offset,
                                   std::string& errMsg);

diff --git a/tools/lto/lto.cpp b/tools/lto/lto.cpp
index 11ad532be896c2f6a77b782f3d006e29c0d66152..e0df81ab20d3148a1796df1edc3c938a7e1fa203 100644 (file)
--- a/tools/lto/lto.cpp
+++ b/tools/lto/lto.cpp
@@ -78,8 +78,7 @@ lto_module_t lto_module_create_from_fd_at_offset(int fd, const char *path,
                                                  size_t file_size,
                                                  size_t map_size,
                                                  off_t offset) {
-  return LTOModule::makeLTOModule(fd, path, file_size, map_size,
-                                  offset, sLastErrorString);
+  return LTOModule::makeLTOModule(fd, path, map_size, offset, sLastErrorString);
 }
 
 /// lto_module_create_from_memory - Loads an object file from memory. Returns

diff --git a/unittests/Support/MemoryBufferTest.cpp b/unittests/Support/MemoryBufferTest.cpp
index 336a0e441c02bf4eb3b2ec30a54bc697a8555e7f..d5ea8de3c1743294a776d3e2dce484cc9e844e55 100644 (file)
--- a/unittests/Support/MemoryBufferTest.cpp
+++ b/unittests/Support/MemoryBufferTest.cpp
@@ -113,13 +113,10 @@ TEST_F(MemoryBufferTest, getOpenFileNoNullTerminator) {
   }
 
   OwningBuffer Buf;
-  error_code EC = MemoryBuffer::getOpenFile(TestFD,
-                                            TestPath.c_str(),
-                                            Buf,
-                                            40000,    // Size
-                                            -1,
-                                            8000,     // Offset
-                                            false);
+  error_code EC = MemoryBuffer::getOpenFileSlice(TestFD, TestPath.c_str(), Buf,
+                                                 40000, // Size
+                                                 8000   // Offset
+                                                 );
   EXPECT_FALSE(EC);
 
   StringRef BufData = Buf->getBuffer();