Index: include/llvm/Support/FileSystem.h =================================================================== --- include/llvm/Support/FileSystem.h +++ include/llvm/Support/FileSystem.h @@ -991,29 +991,27 @@ /// Returns kInvalidFile when the stream is closed. file_t getStderrHandle(); -/// Reads \p Buf.size() bytes from \p FileHandle into \p Buf. The number of -/// bytes actually read is returned in \p BytesRead. On Unix, this is equivalent -/// to `*BytesRead = ::read(FD, Buf.data(), Buf.size())`, with error reporting. -/// BytesRead will contain zero when reaching EOF. +/// Reads \p Buf.size() bytes from \p FileHandle into \p Buf. Returns the number +/// of bytes actually read. On Unix, this is equivalent to `return ::read(FD, +/// Buf.data(), Buf.size())`, with error reporting. Returns 0 when reaching EOF. /// /// @param FileHandle File to read from. /// @param Buf Buffer to read into. -/// @param BytesRead Output parameter of the number of bytes read. -/// @returns The error, if any, or errc::success. -std::error_code readNativeFile(file_t FileHandle, MutableArrayRef Buf, - size_t *BytesRead); +/// @returns The number of bytes read, or error. +Expected readNativeFile(file_t FileHandle, MutableArrayRef Buf); /// Reads \p Buf.size() bytes from \p FileHandle at offset \p Offset into \p /// Buf. If 'pread' is available, this will use that, otherwise it will use -/// 'lseek'. Bytes requested beyond the end of the file will be zero -/// initialized. +/// 'lseek'. Returns the number of bytes actually read. Returns 0 when reaching +/// EOF. /// /// @param FileHandle File to read from. /// @param Buf Buffer to read into. /// @param Offset Offset into the file at which the read should occur. -/// @returns The error, if any, or errc::success. -std::error_code readNativeFileSlice(file_t FileHandle, - MutableArrayRef Buf, size_t Offset); +/// @returns The number of bytes read, or error. +Expected readNativeFileSlice(file_t FileHandle, + MutableArrayRef Buf, + uint64_t Offset); /// @brief Opens the file with the given name in a write-only or read-write /// mode, returning its open file descriptor. If the file does not exist, it Index: lib/Support/MemoryBuffer.cpp =================================================================== --- lib/Support/MemoryBuffer.cpp +++ lib/Support/MemoryBuffer.cpp @@ -211,15 +211,17 @@ 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 @@ 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, Buf->getBuffer(), 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); } Index: lib/Support/Unix/Path.inc =================================================================== --- lib/Support/Unix/Path.inc +++ lib/Support/Unix/Path.inc @@ -999,44 +999,28 @@ 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) { Index: lib/Support/Windows/Path.inc =================================================================== --- lib/Support/Windows/Path.inc +++ lib/Support/Windows/Path.inc @@ -1217,57 +1217,34 @@ 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) { Index: unittests/Support/Path.cpp =================================================================== --- unittests/Support/Path.cpp +++ unittests/Support/Path.cpp @@ -8,6 +8,7 @@ #include "llvm/Support/Path.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/Triple.h" #include "llvm/BinaryFormat/Magic.h" @@ -1514,28 +1515,47 @@ verifyWrite(FD, "Buzz", true); } +TEST_F(FileSystemTest, readNativeFile) { + createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "01234"); + FileRemover Cleanup(NonExistantFile); + const auto &Read = [&](size_t ToRead) -> Expected { + std::string Buf(ToRead, '?'); + Expected FD = fs::openNativeFileForRead(NonExistantFile); + if (!FD) + return FD.takeError(); + auto Close = make_scope_exit([&] { fs::closeFile(*FD); }); + if (Expected BytesRead = fs::readNativeFile( + *FD, makeMutableArrayRef(&*Buf.begin(), Buf.size()))) + return Buf.substr(0, *BytesRead); + else + return BytesRead.takeError(); + }; + EXPECT_THAT_EXPECTED(Read(5), HasValue("01234")); + EXPECT_THAT_EXPECTED(Read(3), HasValue("012")); + EXPECT_THAT_EXPECTED(Read(6), HasValue("01234")); +} + TEST_F(FileSystemTest, readNativeFileSlice) { - char Data[10] = {'0', '1', '2', '3', '4', 0, 0, 0, 0, 0}; - createFileWithData(NonExistantFile, false, fs::CD_CreateNew, - StringRef(Data, 5)); + createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "01234"); FileRemover Cleanup(NonExistantFile); Expected FD = fs::openNativeFileForRead(NonExistantFile); ASSERT_THAT_EXPECTED(FD, Succeeded()); - char Buf[10]; + auto Close = make_scope_exit([&] { fs::closeFile(*FD); }); const auto &Read = [&](size_t Offset, - size_t ToRead) -> Expected> { - std::memset(Buf, 0x47, sizeof(Buf)); - if (std::error_code EC = fs::readNativeFileSlice( - *FD, makeMutableArrayRef(Buf, ToRead), Offset)) - return errorCodeToError(EC); - return makeArrayRef(Buf, ToRead); + size_t ToRead) -> Expected { + std::string Buf(ToRead, '?'); + if (Expected BytesRead = fs::readNativeFileSlice( + *FD, makeMutableArrayRef(&*Buf.begin(), Buf.size()), Offset)) + return Buf.substr(0, *BytesRead); + else + return BytesRead.takeError(); }; - EXPECT_THAT_EXPECTED(Read(0, 5), HasValue(makeArrayRef(Data + 0, 5))); - EXPECT_THAT_EXPECTED(Read(0, 3), HasValue(makeArrayRef(Data + 0, 3))); - EXPECT_THAT_EXPECTED(Read(2, 3), HasValue(makeArrayRef(Data + 2, 3))); - EXPECT_THAT_EXPECTED(Read(0, 6), HasValue(makeArrayRef(Data + 0, 6))); - EXPECT_THAT_EXPECTED(Read(2, 6), HasValue(makeArrayRef(Data + 2, 6))); - EXPECT_THAT_EXPECTED(Read(5, 5), HasValue(makeArrayRef(Data + 5, 5))); + EXPECT_THAT_EXPECTED(Read(0, 5), HasValue("01234")); + EXPECT_THAT_EXPECTED(Read(0, 3), HasValue("012")); + EXPECT_THAT_EXPECTED(Read(2, 3), HasValue("234")); + EXPECT_THAT_EXPECTED(Read(0, 6), HasValue("01234")); + EXPECT_THAT_EXPECTED(Read(2, 6), HasValue("234")); + EXPECT_THAT_EXPECTED(Read(5, 5), HasValue("")); } TEST_F(FileSystemTest, is_local) {