From 1b30ea2c504432bf2d2164ae3882137a43162e07 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 22 Aug 2019 08:13:30 +0000 Subject: [Support] Improve readNativeFile(Slice) interface Summary: There was a subtle, but pretty important difference between the Slice and regular versions of this function. The Slice function was zero-initializing the rest of the buffer when the read syscall returned less bytes than expected, while the regular function did not. This patch removes the inconsistency by making both functions *not* zero-initialize the buffer. The zeroing code is moved to the MemoryBuffer class, which is currently the only user of this code. This makes the API more consistent, and the code shorter. While in there, I also refactor the functions to return the number of bytes through the regular return value (via Expected) instead of a separate by-ref argument. Reviewers: aganea, rnk Subscribers: kristina, Bigcheese, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D66471 llvm-svn: 369627 --- llvm/lib/Support/MemoryBuffer.cpp | 33 +++++++++++------ llvm/lib/Support/Unix/Path.inc | 50 +++++++++----------------- llvm/lib/Support/Windows/Path.inc | 75 ++++++++++++++------------------------- 3 files changed, 66 insertions(+), 92 deletions(-) (limited to 'llvm/lib/Support') diff --git a/llvm/lib/Support/MemoryBuffer.cpp b/llvm/lib/Support/MemoryBuffer.cpp index 2f31f059ddb..e4027ca7bbf 100644 --- a/llvm/lib/Support/MemoryBuffer.cpp +++ b/llvm/lib/Support/MemoryBuffer.cpp @@ -211,15 +211,17 @@ static ErrorOr> getMemoryBufferForStream(sys::fs::file_t FD, const Twine &BufferName) { const ssize_t ChunkSize = 4096*4; SmallString Buffer; - size_t ReadBytes; // Read into Buffer until we hit EOF. - do { + for (;;) { Buffer.reserve(Buffer.size() + ChunkSize); - if (auto EC = sys::fs::readNativeFile( - FD, makeMutableArrayRef(Buffer.end(), ChunkSize), &ReadBytes)) - return EC; - Buffer.set_size(Buffer.size() + ReadBytes); - } while (ReadBytes != 0); + Expected ReadBytes = sys::fs::readNativeFile( + FD, makeMutableArrayRef(Buffer.end(), ChunkSize)); + if (!ReadBytes) + return errorToErrorCode(ReadBytes.takeError()); + if (*ReadBytes == 0) + break; + Buffer.set_size(Buffer.size() + *ReadBytes); + } return getMemBufferCopyImpl(Buffer, BufferName); } @@ -458,9 +460,20 @@ getOpenFileImpl(sys::fs::file_t FD, const Twine &Filename, uint64_t FileSize, return make_error_code(errc::not_enough_memory); } - if (std::error_code EC = - sys::fs::readNativeFileSlice(FD, Buf->getBuffer(), Offset)) - return EC; + // Read until EOF, zero-initialize the rest. + MutableArrayRef ToRead = Buf->getBuffer(); + while (!ToRead.empty()) { + Expected ReadBytes = + sys::fs::readNativeFileSlice(FD, ToRead, Offset); + if (!ReadBytes) + return errorToErrorCode(ReadBytes.takeError()); + if (*ReadBytes == 0) { + std::memset(ToRead.data(), 0, ToRead.size()); + break; + } + ToRead = ToRead.drop_front(*ReadBytes); + Offset += *ReadBytes; + } return std::move(Buf); } diff --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc index cbc03821940..a617eca3566 100644 --- a/llvm/lib/Support/Unix/Path.inc +++ b/llvm/lib/Support/Unix/Path.inc @@ -999,44 +999,28 @@ file_t getStdinHandle() { return 0; } file_t getStdoutHandle() { return 1; } file_t getStderrHandle() { return 2; } -std::error_code readNativeFile(file_t FD, MutableArrayRef Buf, - size_t *BytesRead) { - *BytesRead = sys::RetryAfterSignal(-1, ::read, FD, Buf.data(), Buf.size()); - if (ssize_t(*BytesRead) == -1) - return std::error_code(errno, std::generic_category()); - return std::error_code(); +Expected readNativeFile(file_t FD, MutableArrayRef Buf) { + ssize_t NumRead = + sys::RetryAfterSignal(-1, ::read, FD, Buf.data(), Buf.size()); + if (ssize_t(NumRead) == -1) + return errorCodeToError(std::error_code(errno, std::generic_category())); + return NumRead; } -std::error_code readNativeFileSlice(file_t FD, MutableArrayRef Buf, - size_t Offset) { - char *BufPtr = Buf.data(); - size_t BytesLeft = Buf.size(); - -#ifndef HAVE_PREAD - // If we don't have pread, seek to Offset. - if (lseek(FD, Offset, SEEK_SET) == -1) - return std::error_code(errno, std::generic_category()); -#endif - - while (BytesLeft) { +Expected readNativeFileSlice(file_t FD, MutableArrayRef Buf, + uint64_t Offset) { #ifdef HAVE_PREAD - ssize_t NumRead = sys::RetryAfterSignal(-1, ::pread, FD, BufPtr, BytesLeft, - Buf.size() - BytesLeft + Offset); + ssize_t NumRead = + sys::RetryAfterSignal(-1, ::pread, FD, Buf.data(), Buf.size(), Offset); #else - ssize_t NumRead = sys::RetryAfterSignal(-1, ::read, FD, BufPtr, BytesLeft); + if (lseek(FD, Offset, SEEK_SET) == -1) + return errorCodeToError(std::error_code(errno, std::generic_category())); + ssize_t NumRead = + sys::RetryAfterSignal(-1, ::read, FD, Buf.data(), Buf.size()); #endif - if (NumRead == -1) { - // Error while reading. - return std::error_code(errno, std::generic_category()); - } - if (NumRead == 0) { - memset(BufPtr, 0, BytesLeft); // zero-initialize rest of the buffer. - break; - } - BytesLeft -= NumRead; - BufPtr += NumRead; - } - return std::error_code(); + if (NumRead == -1) + return errorCodeToError(std::error_code(errno, std::generic_category())); + return NumRead; } std::error_code closeFile(file_t &F) { diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc index 00a4e669abb..dd6ea995d23 100644 --- a/llvm/lib/Support/Windows/Path.inc +++ b/llvm/lib/Support/Windows/Path.inc @@ -1217,57 +1217,34 @@ file_t getStdinHandle() { return ::GetStdHandle(STD_INPUT_HANDLE); } file_t getStdoutHandle() { return ::GetStdHandle(STD_OUTPUT_HANDLE); } file_t getStderrHandle() { return ::GetStdHandle(STD_ERROR_HANDLE); } -std::error_code readNativeFileImpl(file_t FileHandle, char *BufPtr, size_t BytesToRead, - size_t *BytesRead, OVERLAPPED *Overlap) { +Expected readNativeFileImpl(file_t FileHandle, + MutableArrayRef Buf, + OVERLAPPED *Overlap) { // ReadFile can only read 2GB at a time. The caller should check the number of // bytes and read in a loop until termination. - DWORD BytesToRead32 = - std::min(size_t(std::numeric_limits::max()), BytesToRead); - DWORD BytesRead32 = 0; - bool Success = - ::ReadFile(FileHandle, BufPtr, BytesToRead32, &BytesRead32, Overlap); - *BytesRead = BytesRead32; - if (!Success) { - DWORD Err = ::GetLastError(); - // EOF is not an error. - if (Err == ERROR_BROKEN_PIPE || Err == ERROR_HANDLE_EOF) - return std::error_code(); - return mapWindowsError(Err); - } - return std::error_code(); -} - -std::error_code readNativeFile(file_t FileHandle, MutableArrayRef Buf, - size_t *BytesRead) { - return readNativeFileImpl(FileHandle, Buf.data(), Buf.size(), BytesRead, - /*Overlap=*/nullptr); -} - -std::error_code readNativeFileSlice(file_t FileHandle, - MutableArrayRef Buf, size_t Offset) { - char *BufPtr = Buf.data(); - size_t BytesLeft = Buf.size(); - - while (BytesLeft) { - uint64_t CurOff = Buf.size() - BytesLeft + Offset; - OVERLAPPED Overlapped = {}; - Overlapped.Offset = uint32_t(CurOff); - Overlapped.OffsetHigh = uint32_t(uint64_t(CurOff) >> 32); - - size_t BytesRead = 0; - if (auto EC = readNativeFileImpl(FileHandle, BufPtr, BytesLeft, &BytesRead, - &Overlapped)) - return EC; - - // Once we reach EOF, zero the remaining bytes in the buffer. - if (BytesRead == 0) { - memset(BufPtr, 0, BytesLeft); - break; - } - BytesLeft -= BytesRead; - BufPtr += BytesRead; - } - return std::error_code(); + DWORD BytesToRead = + std::min(size_t(std::numeric_limits::max()), Buf.size()); + DWORD BytesRead = 0; + if (::ReadFile(FileHandle, Buf.data(), BytesToRead, &BytesRead, Overlap)) + return BytesRead; + DWORD Err = ::GetLastError(); + // EOF is not an error. + if (Err == ERROR_BROKEN_PIPE || Err == ERROR_HANDLE_EOF) + return BytesRead; + return errorCodeToError(mapWindowsError(Err)); +} + +Expected readNativeFile(file_t FileHandle, MutableArrayRef Buf) { + return readNativeFileImpl(FileHandle, Buf, /*Overlap=*/nullptr); +} + +Expected readNativeFileSlice(file_t FileHandle, + MutableArrayRef Buf, + uint64_t Offset) { + OVERLAPPED Overlapped = {}; + Overlapped.Offset = uint32_t(Offset); + Overlapped.OffsetHigh = uint32_t(Offset >> 32); + return readNativeFileImpl(FileHandle, Buf, &Overlapped); } std::error_code closeFile(file_t &F) { -- cgit v1.2.3