From 3d2ac2e41a96f1989a4aa240b8323d09e5cdd397 Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Tue, 23 Jul 2013 20:25:01 +0000 Subject: [PATCH] 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. llvm-svn: 186984 --- llvm/include/llvm/Support/MemoryBuffer.h | 16 +++++++++------ llvm/lib/Support/MemoryBuffer.cpp | 32 ++++++++++++++++++++++------- llvm/tools/gold/gold-plugin.cpp | 5 ++--- llvm/tools/llvm-ar/llvm-ar.cpp | 6 +++--- llvm/tools/lto/LTOModule.cpp | 7 +++---- llvm/tools/lto/LTOModule.h | 1 - llvm/tools/lto/lto.cpp | 3 +-- llvm/unittests/Support/MemoryBufferTest.cpp | 11 ++++------ 8 files changed, 48 insertions(+), 33 deletions(-) diff --git a/llvm/include/llvm/Support/MemoryBuffer.h b/llvm/include/llvm/Support/MemoryBuffer.h index e443b68..9609e33 100644 --- a/llvm/include/llvm/Support/MemoryBuffer.h +++ b/llvm/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 &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 &result, - uint64_t FileSize = -1, - uint64_t MapSize = -1, - int64_t Offset = 0, + OwningPtr &Result, + uint64_t FileSize, bool RequiresNullTerminator = true); /// getMemBuffer - Open the specified memory range as a MemoryBuffer. Note diff --git a/llvm/lib/Support/MemoryBuffer.cpp b/llvm/lib/Support/MemoryBuffer.cpp index 051d64b..cab45c7 100644 --- a/llvm/lib/Support/MemoryBuffer.cpp +++ b/llvm/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 &Result, + uint64_t FileSize, uint64_t MapSize, + int64_t Offset, bool RequiresNullTerminator); + error_code MemoryBuffer::getFile(const char *Filename, OwningPtr &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 &result, - uint64_t FileSize, uint64_t MapSize, - int64_t Offset, - bool RequiresNullTerminator) { +static error_code getOpenFileImpl(int FD, const char *Filename, + OwningPtr &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 &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 &Result, + uint64_t MapSize, int64_t Offset) { + return getOpenFileImpl(FD, Filename, Result, -1, MapSize, Offset, false); +} + //===----------------------------------------------------------------------===// // MemoryBuffer::getSTDIN implementation. //===----------------------------------------------------------------------===// diff --git a/llvm/tools/gold/gold-plugin.cpp b/llvm/tools/gold/gold-plugin.cpp index 133eca4..7771709 100644 --- a/llvm/tools/gold/gold-plugin.cpp +++ b/llvm/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/llvm/tools/llvm-ar/llvm-ar.cpp b/llvm/tools/llvm-ar/llvm-ar.cpp index 116143e..261446c 100644 --- a/llvm/tools/llvm-ar/llvm-ar.cpp +++ b/llvm/tools/llvm-ar/llvm-ar.cpp @@ -770,9 +770,9 @@ static void performWriteOperation(ArchiveOperation Operation, failIfError(sys::fs::status(FD, Status), FileName); OwningPtr 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/llvm/tools/lto/LTOModule.cpp b/llvm/tools/lto/LTOModule.cpp index 5ee43ba..6626eaa 100644 --- a/llvm/tools/lto/LTOModule.cpp +++ b/llvm/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 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/llvm/tools/lto/LTOModule.h b/llvm/tools/lto/LTOModule.h index 83f3a7d..902e9c5 100644 --- a/llvm/tools/lto/LTOModule.h +++ b/llvm/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/llvm/tools/lto/lto.cpp b/llvm/tools/lto/lto.cpp index 11ad532..e0df81a 100644 --- a/llvm/tools/lto/lto.cpp +++ b/llvm/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/llvm/unittests/Support/MemoryBufferTest.cpp b/llvm/unittests/Support/MemoryBufferTest.cpp index 336a0e4..d5ea8de 100644 --- a/llvm/unittests/Support/MemoryBufferTest.cpp +++ b/llvm/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(); -- 2.7.4