[llvm] r186984 - Split getOpenFile into getOpenFile and getOpenFileSlice.

Rafael Espindola rafael.espindola at gmail.com
Tue Jul 23 13:25:02 PDT 2013


Author: rafael
Date: Tue Jul 23 15:25:01 2013
New Revision: 186984

URL: http://llvm.org/viewvc/llvm-project?rev=186984&view=rev
Log:
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.

Modified:
    llvm/trunk/include/llvm/Support/MemoryBuffer.h
    llvm/trunk/lib/Support/MemoryBuffer.cpp
    llvm/trunk/tools/gold/gold-plugin.cpp
    llvm/trunk/tools/llvm-ar/llvm-ar.cpp
    llvm/trunk/tools/lto/LTOModule.cpp
    llvm/trunk/tools/lto/LTOModule.h
    llvm/trunk/tools/lto/lto.cpp
    llvm/trunk/unittests/Support/MemoryBufferTest.cpp

Modified: llvm/trunk/include/llvm/Support/MemoryBuffer.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/MemoryBuffer.h?rev=186984&r1=186983&r2=186984&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/MemoryBuffer.h (original)
+++ llvm/trunk/include/llvm/Support/MemoryBuffer.h Tue Jul 23 15:25:01 2013
@@ -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

Modified: llvm/trunk/lib/Support/MemoryBuffer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/MemoryBuffer.cpp?rev=186984&r1=186983&r2=186984&view=diff
==============================================================================
--- llvm/trunk/lib/Support/MemoryBuffer.cpp (original)
+++ llvm/trunk/lib/Support/MemoryBuffer.cpp Tue Jul 23 15:25:01 2013
@@ -248,6 +248,11 @@ error_code MemoryBuffer::getFile(StringR
                                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 c
   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
   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.
 //===----------------------------------------------------------------------===//

Modified: llvm/trunk/tools/gold/gold-plugin.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/gold/gold-plugin.cpp?rev=186984&r1=186983&r2=186984&view=diff
==============================================================================
--- llvm/trunk/tools/gold/gold-plugin.cpp (original)
+++ llvm/trunk/tools/gold/gold-plugin.cpp Tue Jul 23 15:25:01 2013
@@ -252,9 +252,8 @@ static ld_plugin_status claim_file_hook(
     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;
     }

Modified: llvm/trunk/tools/llvm-ar/llvm-ar.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-ar/llvm-ar.cpp?rev=186984&r1=186983&r2=186984&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-ar/llvm-ar.cpp (original)
+++ llvm/trunk/tools/llvm-ar/llvm-ar.cpp Tue Jul 23 15:25:01 2013
@@ -770,9 +770,9 @@ static void performWriteOperation(Archiv
       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)

Modified: llvm/trunk/tools/lto/LTOModule.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/lto/LTOModule.cpp?rev=186984&r1=186983&r2=186984&view=diff
==============================================================================
--- llvm/trunk/tools/lto/LTOModule.cpp (original)
+++ llvm/trunk/tools/lto/LTOModule.cpp Tue Jul 23 15:25:01 2013
@@ -209,17 +209,16 @@ LTOModule *LTOModule::makeLTOModule(cons
 
 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;
   }

Modified: llvm/trunk/tools/lto/LTOModule.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/lto/LTOModule.h?rev=186984&r1=186983&r2=186984&view=diff
==============================================================================
--- llvm/trunk/tools/lto/LTOModule.h (original)
+++ llvm/trunk/tools/lto/LTOModule.h Tue Jul 23 15:25:01 2013
@@ -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);

Modified: llvm/trunk/tools/lto/lto.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/lto/lto.cpp?rev=186984&r1=186983&r2=186984&view=diff
==============================================================================
--- llvm/trunk/tools/lto/lto.cpp (original)
+++ llvm/trunk/tools/lto/lto.cpp Tue Jul 23 15:25:01 2013
@@ -78,8 +78,7 @@ lto_module_t lto_module_create_from_fd_a
                                                  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

Modified: llvm/trunk/unittests/Support/MemoryBufferTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/MemoryBufferTest.cpp?rev=186984&r1=186983&r2=186984&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/MemoryBufferTest.cpp (original)
+++ llvm/trunk/unittests/Support/MemoryBufferTest.cpp Tue Jul 23 15:25:01 2013
@@ -113,13 +113,10 @@ TEST_F(MemoryBufferTest, getOpenFileNoNu
   }
 
   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();





More information about the llvm-commits mailing list