Index: clang-tools-extra/clang-move/tool/ClangMoveMain.cpp =================================================================== --- clang-tools-extra/clang-move/tool/ClangMoveMain.cpp +++ clang-tools-extra/clang-move/tool/ClangMoveMain.cpp @@ -30,8 +30,8 @@ std::error_code CreateNewFile(const llvm::Twine &path) { int fd = 0; - if (std::error_code ec = - llvm::sys::fs::openFileForWrite(path, fd, llvm::sys::fs::F_Text)) + if (std::error_code ec = llvm::sys::fs::openFileForWrite( + path, fd, llvm::sys::fs::CD_CreateAlways, llvm::sys::fs::F_Text)) return ec; return llvm::sys::Process::SafelyCloseFileDescriptor(fd); Index: clang-tools-extra/clangd/tool/ClangdMain.cpp =================================================================== --- clang-tools-extra/clangd/tool/ClangdMain.cpp +++ clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -159,7 +159,8 @@ llvm::Optional InputMirrorStream; if (!InputMirrorFile.empty()) { std::error_code EC; - InputMirrorStream.emplace(InputMirrorFile, /*ref*/ EC, llvm::sys::fs::F_RW); + InputMirrorStream.emplace(InputMirrorFile, /*ref*/ EC, + llvm::sys::fs::FA_Read | llvm::sys::fs::FA_Write); if (EC) { InputMirrorStream.reset(); llvm::errs() << "Error while opening an input mirror file: " @@ -174,7 +175,8 @@ std::unique_ptr Tracer; if (auto *TraceFile = getenv("CLANGD_TRACE")) { std::error_code EC; - TraceStream.emplace(TraceFile, /*ref*/ EC, llvm::sys::fs::F_RW); + TraceStream.emplace(TraceFile, /*ref*/ EC, + llvm::sys::fs::FA_Read | llvm::sys::fs::FA_Write); if (EC) { TraceStream.reset(); llvm::errs() << "Error while opening trace file " << TraceFile << ": " Index: clang/lib/Basic/VirtualFileSystem.cpp =================================================================== --- clang/lib/Basic/VirtualFileSystem.cpp +++ clang/lib/Basic/VirtualFileSystem.cpp @@ -258,7 +258,8 @@ RealFileSystem::openFileForRead(const Twine &Name) { int FD; SmallString<256> RealName; - if (std::error_code EC = sys::fs::openFileForRead(Name, FD, &RealName)) + if (std::error_code EC = + sys::fs::openFileForRead(Name, FD, sys::fs::OF_None, &RealName)) return EC; return std::unique_ptr(new RealFile(FD, Name.str(), RealName.str())); } Index: clang/lib/Driver/Driver.cpp =================================================================== --- clang/lib/Driver/Driver.cpp +++ clang/lib/Driver/Driver.cpp @@ -1293,7 +1293,7 @@ std::string Script = CrashInfo.Filename.rsplit('.').first.str() + ".sh"; std::error_code EC; - llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::F_Excl); + llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::CD_CreateNew); if (EC) { Diag(clang::diag::note_drv_command_failed_diag_msg) << "Error generating run script: " + Script + " " + EC.message(); Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -238,10 +238,8 @@ << "-" << i << ".html"; llvm::sys::path::append(Model, Directory, filename.str()); - EC = llvm::sys::fs::openFileForWrite(Model, - FD, - llvm::sys::fs::F_RW | - llvm::sys::fs::F_Excl); + EC = llvm::sys::fs::openFileForReadWrite( + Model, FD, llvm::sys::fs::CD_CreateNew, llvm::sys::fs::OF_None); if (EC && EC != llvm::errc::file_exists) { llvm::errs() << "warning: could not create file '" << Model << "': " << EC.message() << '\n'; Index: lldb/source/Core/Debugger.cpp =================================================================== --- lldb/source/Core/Debugger.cpp +++ lldb/source/Core/Debugger.cpp @@ -1241,8 +1241,8 @@ if (log_options & LLDB_LOG_OPTION_APPEND) flags |= llvm::sys::fs::F_Append; int FD; - if (std::error_code ec = - llvm::sys::fs::openFileForWrite(log_file, FD, flags)) { + if (std::error_code ec = llvm::sys::fs::openFileForWrite( + log_file, FD, llvm::sys::fs::CD_CreateAlways, flags)) { error_stream << "Unable to open log file: " << ec.message(); return false; } Index: llvm/include/llvm/Support/FileSystem.h =================================================================== --- llvm/include/llvm/Support/FileSystem.h +++ llvm/include/llvm/Support/FileSystem.h @@ -676,32 +676,48 @@ /// platform-specific error_code. std::error_code status_known(const Twine &path, bool &result); -enum OpenFlags : unsigned { - F_None = 0, - - /// F_Excl - When opening a file, this flag makes raw_fd_ostream - /// report an error if the file already exists. - F_Excl = 1, +enum CreationDisposition : unsigned { + /// CD_CreateAlways - When opening a file: + /// * If it already exists, truncate it. + /// * If it does not already exist, create a new file. + CD_CreateAlways = 0, + + /// CD_CreateNew - When opening a file: + /// * If it already exists, fail. + /// * If it does not already exist, create a new file. + CD_CreateNew = 1, + + /// CD_OpenAlways - When opening a file: + /// * If it already exists, open the file with the offset set to 0. + /// * If it does not already exist, fail. + CD_OpenExisting = 2, + + /// CD_OpenAlways - When opening a file: + /// * If it already exists, open the file with the offset set to 0. + /// * If it does not already exist, create a new file. + CD_OpenAlways = 3, +}; - /// F_Append - When opening a file, if it already exists append to the - /// existing file instead of returning an error. This may not be specified - /// with F_Excl. - F_Append = 2, +enum FileAccess : unsigned { + FA_Read = 1, + FA_Write = 2, +}; - /// F_NoTrunc - When opening a file, if it already exists don't truncate - /// the file contents. F_Append implies F_NoTrunc, but F_Append seeks to - /// the end of the file, which F_NoTrunc doesn't. - F_NoTrunc = 4, +enum OpenFlags : unsigned { + OF_None = 0, + F_None = 0, // For compatibility /// The file should be opened in text mode on platforms that make this /// distinction. - F_Text = 8, + OF_Text = 1, + F_Text = 1, // For compatibility - /// Open the file for read and write. - F_RW = 16, + /// The file should be opened in append mode. + OF_Append = 2, + F_Append = 2, // For compatibility /// Delete the file on close. Only makes a difference on windows. - F_Delete = 32 + OF_Delete = 4 }; /// Create a uniquely named file. @@ -824,6 +840,15 @@ return A; } +inline FileAccess operator|(FileAccess A, FileAccess B) { + return FileAccess(unsigned(A) | unsigned(B)); +} + +inline FileAccess &operator|=(FileAccess &A, FileAccess B) { + A = A | B; + return A; +} + /// @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 /// is created. @@ -840,7 +865,45 @@ /// @returns errc::success if \a Name has been opened, otherwise a /// platform-specific error_code. std::error_code openFileForWrite(const Twine &Name, int &ResultFD, - OpenFlags Flags, unsigned Mode = 0666); + CreationDisposition Disp = CD_CreateAlways, + OpenFlags Flags = OF_None, + unsigned Mode = 0666); + +/// @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 +/// is created. +/// +/// The caller is responsible for closing the freeing the file once they are +/// finished with it. +/// +/// @param Name The path of the file to open, relative or absolute. +/// @param Flags Additional flags used to determine whether the file should be +/// opened in, for example, read-write or in write-only mode. +/// @param Mode The access permissions of the file, represented in octal. +/// @returns a platform-specific file descriptor if \a Name has been opened, +/// otherwise an error object. +Expected openNativeFileForWrite(const Twine &Name, + CreationDisposition Disp, + OpenFlags Flags, unsigned Mode = 0666); + +/// @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 +/// is created. +/// +/// The caller is responsible for closing the file descriptor once they are +/// finished with it. +/// +/// @param Name The path of the file to open, relative or absolute. +/// @param ResultFD If the file could be opened successfully, its descriptor +/// is stored in this location. Otherwise, this is set to -1. +/// @param Flags Additional flags used to determine whether the file should be +/// opened in, for example, read-write or in write-only mode. +/// @param Mode The access permissions of the file, represented in octal. +/// @returns errc::success if \a Name has been opened, otherwise a +/// platform-specific error_code. +std::error_code openFileForReadWrite(const Twine &Name, int &ResultFD, + CreationDisposition Disp, OpenFlags Flags, + unsigned Mode = 0666); /// @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 @@ -855,8 +918,10 @@ /// @param Mode The access permissions of the file, represented in octal. /// @returns a platform-specific file descriptor if \a Name has been opened, /// otherwise an error object. -Expected openNativeFileForWrite(const Twine &Name, OpenFlags Flags, - unsigned Mode = 0666); +Expected openNativeFileForReadWrite(const Twine &Name, + CreationDisposition Disp, + OpenFlags Flags, + unsigned Mode = 0666); /// @brief Opens the file with the given name in a read-only mode, returning /// its open file descriptor. @@ -873,6 +938,7 @@ /// @returns errc::success if \a Name has been opened, otherwise a /// platform-specific error_code. std::error_code openFileForRead(const Twine &Name, int &ResultFD, + OpenFlags Flags = OF_None, SmallVectorImpl *RealPath = nullptr); /// @brief Opens the file with the given name in a read-only mode, returning @@ -888,7 +954,7 @@ /// @returns a platform-specific file descriptor if \a Name has been opened, /// otherwise an error object. Expected -openNativeFileForRead(const Twine &Name, +openNativeFileForRead(const Twine &Name, OpenFlags Flags = OF_None, SmallVectorImpl *RealPath = nullptr); /// @brief Close the file object. This should be used instead of ::close for Index: llvm/include/llvm/Support/raw_ostream.h =================================================================== --- llvm/include/llvm/Support/raw_ostream.h +++ llvm/include/llvm/Support/raw_ostream.h @@ -33,7 +33,9 @@ namespace sys { namespace fs { +enum FileAccess : unsigned; enum OpenFlags : unsigned; +enum CreationDisposition : unsigned; } // end namespace fs } // end namespace sys @@ -397,7 +399,15 @@ /// As a special case, if Filename is "-", then the stream will use /// STDOUT_FILENO instead of opening a file. This will not close the stdout /// descriptor. + raw_fd_ostream(StringRef Filename, std::error_code &EC); raw_fd_ostream(StringRef Filename, std::error_code &EC, + sys::fs::CreationDisposition Disp); + raw_fd_ostream(StringRef Filename, std::error_code &EC, + sys::fs::FileAccess Access); + raw_fd_ostream(StringRef Filename, std::error_code &EC, + sys::fs::OpenFlags Flags); + raw_fd_ostream(StringRef Filename, std::error_code &EC, + sys::fs::CreationDisposition Disp, sys::fs::FileAccess Access, sys::fs::OpenFlags Flags); /// FD is the file descriptor that this writes to. If ShouldClose is true, Index: llvm/lib/Support/FileOutputBuffer.cpp =================================================================== --- llvm/lib/Support/FileOutputBuffer.cpp +++ llvm/lib/Support/FileOutputBuffer.cpp @@ -82,9 +82,10 @@ size_t getBufferSize() const override { return Buffer.size(); } Error commit() override { + using namespace sys::fs; int FD; std::error_code EC; - if (auto EC = openFileForWrite(FinalPath, FD, fs::F_None, Mode)) + if (auto EC = openFileForWrite(FinalPath, FD, CD_CreateAlways, OF_None)) return errorCodeToError(EC); raw_fd_ostream OS(FD, /*shouldClose=*/true, /*unbuffered=*/true); OS << StringRef((const char *)Buffer.base(), Buffer.size()); Index: llvm/lib/Support/MemoryBuffer.cpp =================================================================== --- llvm/lib/Support/MemoryBuffer.cpp +++ llvm/lib/Support/MemoryBuffer.cpp @@ -244,7 +244,7 @@ getFileAux(const Twine &Filename, int64_t FileSize, uint64_t MapSize, uint64_t Offset, bool RequiresNullTerminator, bool IsVolatile) { int FD; - std::error_code EC = sys::fs::openFileForRead(Filename, FD); + std::error_code EC = sys::fs::openFileForRead(Filename, FD, sys::fs::OF_None); if (EC) return EC; @@ -364,8 +364,8 @@ getReadWriteFile(const Twine &Filename, uint64_t FileSize, uint64_t MapSize, uint64_t Offset) { int FD; - std::error_code EC = sys::fs::openFileForWrite( - Filename, FD, sys::fs::F_RW | sys::fs::F_NoTrunc); + std::error_code EC = sys::fs::openFileForReadWrite( + Filename, FD, sys::fs::CD_OpenExisting, sys::fs::OF_None); if (EC) return EC; @@ -518,7 +518,7 @@ ErrorOr> MemoryBuffer::getFileAsStream(const Twine &Filename) { int FD; - std::error_code EC = sys::fs::openFileForRead(Filename, FD); + std::error_code EC = sys::fs::openFileForRead(Filename, FD, sys::fs::OF_None); if (EC) return EC; ErrorOr> Ret = Index: llvm/lib/Support/Path.cpp =================================================================== --- llvm/lib/Support/Path.cpp +++ llvm/lib/Support/Path.cpp @@ -169,7 +169,7 @@ createUniqueEntity(const Twine &Model, int &ResultFD, SmallVectorImpl &ResultPath, bool MakeAbsolute, unsigned Mode, FSEntity Type, - sys::fs::OpenFlags Flags = sys::fs::F_RW) { + sys::fs::OpenFlags Flags = sys::fs::OF_None) { SmallString<128> ModelStorage; Model.toVector(ModelStorage); @@ -201,8 +201,8 @@ switch (Type) { case FS_File: { if (std::error_code EC = - sys::fs::openFileForWrite(Twine(ResultPath.begin()), ResultFD, - Flags | sys::fs::F_Excl, Mode)) { + sys::fs::openFileForReadWrite(Twine(ResultPath.begin()), ResultFD, + sys::fs::CD_CreateNew, Flags, Mode)) { if (EC == errc::file_exists) goto retry_random_path; return EC; @@ -929,9 +929,10 @@ std::error_code copy_file(const Twine &From, const Twine &To) { int ReadFD, WriteFD; - if (std::error_code EC = openFileForRead(From, ReadFD)) + if (std::error_code EC = openFileForRead(From, ReadFD, OF_None)) return EC; - if (std::error_code EC = openFileForWrite(To, WriteFD, F_None)) { + if (std::error_code EC = + openFileForWrite(To, WriteFD, CD_CreateAlways, OF_None)) { close(ReadFD); return EC; } @@ -983,7 +984,7 @@ ErrorOr md5_contents(const Twine &Path) { int FD; - if (auto EC = openFileForRead(Path, FD)) + if (auto EC = openFileForRead(Path, FD, OF_None)) return EC; auto Result = md5_contents(FD); @@ -1180,7 +1181,7 @@ int FD; SmallString<128> ResultPath; if (std::error_code EC = - createUniqueFile(Model, FD, ResultPath, Mode, F_Delete | F_RW)) + createUniqueFile(Model, FD, ResultPath, Mode, OF_Delete)) return errorCodeToError(EC); TempFile Ret(ResultPath, FD); Index: llvm/lib/Support/TarWriter.cpp =================================================================== --- llvm/lib/Support/TarWriter.cpp +++ llvm/lib/Support/TarWriter.cpp @@ -159,8 +159,10 @@ // Creates a TarWriter instance and returns it. Expected> TarWriter::create(StringRef OutputPath, StringRef BaseDir) { + using namespace sys::fs; int FD; - if (std::error_code EC = openFileForWrite(OutputPath, FD, sys::fs::F_None)) + if (std::error_code EC = + openFileForWrite(OutputPath, FD, CD_CreateAlways, OF_None)) return make_error("cannot open " + OutputPath, EC); return std::unique_ptr(new TarWriter(FD, BaseDir)); } Index: llvm/lib/Support/Unix/Path.inc =================================================================== --- llvm/lib/Support/Unix/Path.inc +++ llvm/lib/Support/Unix/Path.inc @@ -722,21 +722,69 @@ } #endif -std::error_code openFileForRead(const Twine &Name, int &ResultFD, - SmallVectorImpl *RealPath) { - SmallString<128> Storage; - StringRef P = Name.toNullTerminatedStringRef(Storage); - int OpenFlags = O_RDONLY; +static int nativeOpenFlags(CreationDisposition Disp, OpenFlags Flags, + FileAccess Access) { + int Result = 0; + if (Access == FA_Read) + Result |= O_RDONLY; + else if (Access == FA_Write) + Result |= O_WRONLY; + else if (Access == (FA_Read | FA_Write)) + Result |= O_RDWR; + + // This is for compatibility with old code that assumed F_Append implied + // would open an existing file. See Windows/Path.inc for a longer comment. + if (Flags & F_Append) + Disp = CD_OpenAlways; + + if (Disp == CD_CreateNew) { + Result |= O_CREAT; // Create if it doesn't exist. + Result |= O_EXCL; // Fail if it does. + } else if (Disp == CD_CreateAlways) { + Result |= O_CREAT; // Create if it doesn't exist. + Result |= O_TRUNC; // Truncate if it does. + } else if (Disp == CD_OpenAlways) { + Result |= O_CREAT; // Create if it doesn't exist. + } else if (Disp == CD_OpenExisting) { + // Nothing special, just don't add O_CREAT and we get these semantics. + } + + if (Flags & F_Append) + Result |= O_APPEND; + #ifdef O_CLOEXEC - OpenFlags |= O_CLOEXEC; + Result |= O_CLOEXEC; #endif - if ((ResultFD = sys::RetryAfterSignal(-1, open, P.begin(), OpenFlags)) < 0) + + return Result; +} + +static std::error_code openFile(const Twine &Name, int &ResultFD, + CreationDisposition Disp, FileAccess Access, + OpenFlags Flags, unsigned Mode) { + int OpenFlags = nativeOpenFlags(Disp, Flags, Access); + + SmallString<128> Storage; + StringRef P = Name.toNullTerminatedStringRef(Storage); + if ((ResultFD = sys::RetryAfterSignal(-1, open, P.begin(), OpenFlags, Mode)) < + 0) return std::error_code(errno, std::generic_category()); #ifndef O_CLOEXEC int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC); (void)r; assert(r == 0 && "fcntl(F_SETFD, FD_CLOEXEC) failed"); #endif + return std::error_code(); +} + +std::error_code openFileForRead(const Twine &Name, int &ResultFD, + OpenFlags Flags, + SmallVectorImpl *RealPath) { + std::error_code EC = + openFile(Name, ResultFD, CD_OpenExisting, FA_Read, Flags, 0666); + if (EC) + return EC; + // Attempt to get the real name of the file, if the user asked if(!RealPath) return std::error_code(); @@ -756,6 +804,9 @@ if (CharCount > 0) RealPath->append(Buffer, Buffer + CharCount); } else { + SmallString<128> Storage; + StringRef P = Name.toNullTerminatedStringRef(Storage); + // Use ::realpath to get the real path name if (::realpath(P.begin(), Buffer) != nullptr) RealPath->append(Buffer, Buffer + strlen(Buffer)); @@ -764,56 +815,42 @@ return std::error_code(); } -Expected openNativeFileForRead(const Twine &Name, +Expected openNativeFileForRead(const Twine &Name, OpenFlags Flags, SmallVectorImpl *RealPath) { file_t ResultFD; - std::error_code EC = openFileForRead(Name, ResultFD, RealPath); + std::error_code EC = openFileForRead(Name, ResultFD, Flags, RealPath); if (EC) return errorCodeToError(EC); return ResultFD; } std::error_code openFileForWrite(const Twine &Name, int &ResultFD, - sys::fs::OpenFlags Flags, unsigned Mode) { - // Verify that we don't have both "append" and "excl". - assert((!(Flags & sys::fs::F_Excl) || !(Flags & sys::fs::F_Append)) && - "Cannot specify both 'excl' and 'append' file creation flags!"); - - int OpenFlags = O_CREAT; - -#ifdef O_CLOEXEC - OpenFlags |= O_CLOEXEC; -#endif - - if (Flags & F_RW) - OpenFlags |= O_RDWR; - else - OpenFlags |= O_WRONLY; - - if (Flags & F_Append) - OpenFlags |= O_APPEND; - else if (!(Flags & F_NoTrunc)) - OpenFlags |= O_TRUNC; + CreationDisposition Disp, OpenFlags Flags, + unsigned Mode) { + return openFile(Name, ResultFD, Disp, FA_Write, Flags, Mode); +} - if (Flags & F_Excl) - OpenFlags |= O_EXCL; +Expected openNativeFileForWrite(const Twine &Name, + CreationDisposition Disp, + OpenFlags Flags, unsigned Mode) { + file_t ResultFD; + std::error_code EC = openFileForWrite(Name, ResultFD, Disp, Flags, Mode); + if (EC) + return errorCodeToError(EC); + return ResultFD; +} - SmallString<128> Storage; - StringRef P = Name.toNullTerminatedStringRef(Storage); - if ((ResultFD = sys::RetryAfterSignal(-1, open, P.begin(), OpenFlags, Mode)) < 0) - return std::error_code(errno, std::generic_category()); -#ifndef O_CLOEXEC - int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC); - (void)r; - assert(r == 0 && "fcntl(F_SETFD, FD_CLOEXEC) failed"); -#endif - return std::error_code(); +std::error_code openFileForReadWrite(const Twine &Name, int &ResultFD, + CreationDisposition Disp, OpenFlags Flags, + unsigned Mode) { + return openFile(Name, ResultFD, Disp, FA_Read | FA_Write, Flags, Mode); } -Expected openNativeFileForWrite(const Twine &Name, OpenFlags Flags, - unsigned Mode) { +Expected openNativeFileForReadWrite(const Twine &Name, + CreationDisposition Disp, + OpenFlags Flags, unsigned Mode) { file_t ResultFD; - std::error_code EC = openFileForWrite(Name, ResultFD, Flags, Mode); + std::error_code EC = openFileForReadWrite(Name, ResultFD, Disp, Flags, Mode); if (EC) return errorCodeToError(EC); return ResultFD; Index: llvm/lib/Support/Windows/Path.inc =================================================================== --- llvm/lib/Support/Windows/Path.inc +++ llvm/lib/Support/Windows/Path.inc @@ -1035,31 +1035,20 @@ return Status; } -static std::error_code directoryRealPath(const Twine &Name, - SmallVectorImpl &RealPath) { - SmallVector PathUTF16; +static std::error_code nativeFileToFd(Expected H, int &ResultFD, + OpenFlags Flags) { + int CrtOpenFlags = 0; + if (Flags & OF_Append) + CrtOpenFlags |= _O_APPEND; - if (std::error_code EC = widenPath(Name, PathUTF16)) - return EC; + if (Flags & OF_Text) + CrtOpenFlags |= _O_TEXT; - HANDLE H = - ::CreateFileW(PathUTF16.begin(), GENERIC_READ, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); - if (H == INVALID_HANDLE_VALUE) - return mapWindowsError(GetLastError()); - std::error_code EC = realPathFromHandle(H, RealPath); - ::CloseHandle(H); - return EC; -} - -static std::error_code nativeFileToFd(Expected H, int &ResultFD, - int OpenFlags) { ResultFD = -1; if (!H) return errorToErrorCode(H.takeError()); - ResultFD = ::_open_osfhandle(intptr_t(*H), OpenFlags); + ResultFD = ::_open_osfhandle(intptr_t(*H), CrtOpenFlags); if (ResultFD == -1) { ::CloseHandle(*H); return mapWindowsError(ERROR_INVALID_HANDLE); @@ -1067,23 +1056,62 @@ return std::error_code(); } -std::error_code openFileForRead(const Twine &Name, int &ResultFD, - SmallVectorImpl *RealPath) { - Expected NativeFile = openNativeFileForRead(Name, RealPath); - return nativeFileToFd(std::move(NativeFile), ResultFD, 0); +static DWORD nativeOpenFlags(OpenFlags Flags) { + DWORD Result = 0; + if (Flags & OF_Delete) + Result |= FILE_FLAG_DELETE_ON_CLOSE; + + if (Result == 0) + Result = FILE_ATTRIBUTE_NORMAL; + return Result; +} + +static DWORD nativeDisposition(CreationDisposition Disp, OpenFlags Flags) { + // This is a compatibility hack. Really we should respect the creation + // disposition, but a lot of old code relied on the implicit assumption that + // OF_Append implied it would open an existing file. Since the disposition is + // now explicit and defaults to CD_CreateAlways, this assumption would cause + // any usage of OF_Append to append to a new file, even if the file already + // existed. A better solution might have two new creation dispositions: + // CD_AppendAlways and CD_AppendNew. This would also address the problem of + // OF_Append being used on a read-only descriptor, which doesn't make sense. + if (Flags & OF_Append) + return OPEN_ALWAYS; + + switch (Disp) { + case CD_CreateAlways: + return CREATE_ALWAYS; + case CD_CreateNew: + return CREATE_NEW; + case CD_OpenAlways: + return OPEN_ALWAYS; + case CD_OpenExisting: + return OPEN_EXISTING; + } + llvm_unreachable("unreachable!"); } -Expected openNativeFileForRead(const Twine &Name, - SmallVectorImpl *RealPath) { - SmallVector PathUTF16; +static DWORD nativeAccess(FileAccess Access, OpenFlags Flags) { + DWORD Result = 0; + if (Access & FA_Read) + Result |= GENERIC_READ; + if (Access & FA_Write) + Result |= GENERIC_WRITE; + if (Flags & OF_Delete) + Result |= DELETE; + return Result; +} +static Expected nativeOpenFile(const Twine &Name, DWORD Disp, + DWORD Access, DWORD Flags) { + SmallVector PathUTF16; if (std::error_code EC = widenPath(Name, PathUTF16)) return errorCodeToError(EC); HANDLE H = - ::CreateFileW(PathUTF16.begin(), GENERIC_READ, + ::CreateFileW(PathUTF16.begin(), Access, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + NULL, Disp, Flags, NULL); if (H == INVALID_HANDLE_VALUE) { DWORD LastError = ::GetLastError(); std::error_code EC = mapWindowsError(LastError); @@ -1096,74 +1124,84 @@ return errorCodeToError(make_error_code(errc::is_a_directory)); return errorCodeToError(EC); } + return H; +} - // Fetch the real name of the file, if the user asked - if (RealPath) - realPathFromHandle(H, *RealPath); +static Expected openFile(const Twine &Name, CreationDisposition Disp, + FileAccess Access, OpenFlags Flags) { + // Verify that we don't have both "append" and "excl". + assert((!(Disp == CD_CreateNew) || !(Flags & OF_Append)) && + "Cannot specify both 'CreateNew' and 'Append' file creation flags!"); - return H; + DWORD NativeFlags = nativeOpenFlags(Flags); + DWORD NativeDisp = nativeDisposition(Disp, Flags); + DWORD NativeAccess = nativeAccess(Access, Flags); + + return nativeOpenFile(Name, NativeDisp, NativeAccess, NativeFlags); } -std::error_code openFileForWrite(const Twine &Name, int &ResultFD, - sys::fs::OpenFlags Flags, unsigned Mode) { - int OpenFlags = 0; - if (Flags & F_Append) - OpenFlags |= _O_APPEND; +static std::error_code directoryRealPath(const Twine &Name, + SmallVectorImpl &RealPath) { + Expected EF = nativeOpenFile(Name, OPEN_EXISTING, GENERIC_READ, + FILE_FLAG_BACKUP_SEMANTICS); + if (!EF) + return errorToErrorCode(EF.takeError()); - if (Flags & F_Text) - OpenFlags |= _O_TEXT; + std::error_code EC = realPathFromHandle(*EF, RealPath); + ::CloseHandle(*EF); + return EC; +} - Expected NativeFile = openNativeFileForWrite(Name, Flags, Mode); - return nativeFileToFd(std::move(NativeFile), ResultFD, OpenFlags); +std::error_code openFileForRead(const Twine &Name, int &ResultFD, + OpenFlags Flags, + SmallVectorImpl *RealPath) { + Expected NativeFile = openNativeFileForRead(Name, Flags, RealPath); + return nativeFileToFd(std::move(NativeFile), ResultFD, OF_None); } -Expected openNativeFileForWrite(const Twine &Name, OpenFlags Flags, - unsigned Mode) { - // Verify that we don't have both "append" and "excl". - assert((!(Flags & sys::fs::F_Excl) || !(Flags & sys::fs::F_Append)) && - "Cannot specify both 'excl' and 'append' file creation flags!"); +Expected openNativeFileForRead(const Twine &Name, OpenFlags Flags, + SmallVectorImpl *RealPath) { - SmallVector PathUTF16; + Expected Result = openFile(Name, CD_OpenExisting, FA_Read, Flags); - if (std::error_code EC = widenPath(Name, PathUTF16)) - return errorCodeToError(EC); + // Fetch the real name of the file, if the user asked + if (Result && RealPath) + realPathFromHandle(*Result, *RealPath); - DWORD CreationDisposition; - if (Flags & F_Excl) - CreationDisposition = CREATE_NEW; - else if ((Flags & F_Append) || (Flags & F_NoTrunc)) - CreationDisposition = OPEN_ALWAYS; - else - CreationDisposition = CREATE_ALWAYS; - - DWORD Access = GENERIC_WRITE; - DWORD Attributes = FILE_ATTRIBUTE_NORMAL; - if (Flags & F_RW) - Access |= GENERIC_READ; - if (Flags & F_Delete) { - Access |= DELETE; - Attributes |= FILE_FLAG_DELETE_ON_CLOSE; - } + return std::move(Result); +} - HANDLE H = - ::CreateFileW(PathUTF16.data(), Access, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - NULL, CreationDisposition, Attributes, NULL); +std::error_code openFileForWrite(const Twine &Name, int &ResultFD, + CreationDisposition Disp, OpenFlags Flags, + unsigned Mode) { + Expected NativeFile = openNativeFileForWrite(Name, Disp, Flags, Mode); + if (!NativeFile) + return errorToErrorCode(NativeFile.takeError()); - if (H == INVALID_HANDLE_VALUE) { - DWORD LastError = ::GetLastError(); - std::error_code EC = mapWindowsError(LastError); - // Provide a better error message when trying to open directories. - // This only runs if we failed to open the file, so there is probably - // no performances issues. - if (LastError != ERROR_ACCESS_DENIED) - return errorCodeToError(EC); - if (is_directory(Name)) - return errorCodeToError(make_error_code(errc::is_a_directory)); - return errorCodeToError(EC); - } + return nativeFileToFd(std::move(NativeFile), ResultFD, Flags); +} - return H; +Expected openNativeFileForWrite(const Twine &Name, + CreationDisposition Disp, + OpenFlags Flags, unsigned Mode) { + return openFile(Name, Disp, FA_Write, Flags); +} + +std::error_code openFileForReadWrite(const Twine &Name, int &ResultFD, + CreationDisposition Disp, OpenFlags Flags, + unsigned Mode) { + Expected NativeFile = + openNativeFileForReadWrite(Name, Disp, Flags, Mode); + if (!NativeFile) + return errorToErrorCode(NativeFile.takeError()); + + return nativeFileToFd(std::move(NativeFile), ResultFD, Flags); +} + +Expected openNativeFileForReadWrite(const Twine &Name, + CreationDisposition Disp, + OpenFlags Flags, unsigned Mode) { + return openFile(Name, Disp, FA_Write | FA_Read, Flags); } void closeFile(file_t &F) { @@ -1239,7 +1277,8 @@ return directoryRealPath(path, dest); int fd; - if (std::error_code EC = llvm::sys::fs::openFileForRead(path, fd, &dest)) + if (std::error_code EC = + llvm::sys::fs::openFileForRead(path, fd, OF_None, &dest)) return EC; ::close(fd); return std::error_code(); Index: llvm/lib/Support/Windows/Program.inc =================================================================== --- llvm/lib/Support/Windows/Program.inc +++ llvm/lib/Support/Windows/Program.inc @@ -496,7 +496,7 @@ llvm::sys::writeFileWithEncoding(StringRef FileName, StringRef Contents, WindowsEncodingMethod Encoding) { std::error_code EC; - llvm::raw_fd_ostream OS(FileName, EC, llvm::sys::fs::OpenFlags::F_Text); + llvm::raw_fd_ostream OS(FileName, EC, llvm::sys::fs::F_Text); if (EC) return EC; Index: llvm/lib/Support/raw_ostream.cpp =================================================================== --- llvm/lib/Support/raw_ostream.cpp +++ llvm/lib/Support/raw_ostream.cpp @@ -498,29 +498,56 @@ //===----------------------------------------------------------------------===// static int getFD(StringRef Filename, std::error_code &EC, + sys::fs::CreationDisposition Disp, sys::fs::FileAccess Access, sys::fs::OpenFlags Flags) { + assert((Access & sys::fs::FA_Write) && + "Cannot make a raw_ostream from a read-only descriptor!"); + // Handle "-" as stdout. Note that when we do this, we consider ourself // the owner of stdout and may set the "binary" flag globally based on Flags. if (Filename == "-") { EC = std::error_code(); // If user requested binary then put stdout into binary mode if // possible. - if (!(Flags & sys::fs::F_Text)) + if (!(Flags & sys::fs::OF_Text)) sys::ChangeStdoutToBinary(); return STDOUT_FILENO; } int FD; - EC = sys::fs::openFileForWrite(Filename, FD, Flags); + if (Access & sys::fs::FA_Read) + EC = sys::fs::openFileForReadWrite(Filename, FD, Disp, Flags); + else + EC = sys::fs::openFileForWrite(Filename, FD, Disp, Flags); if (EC) return -1; return FD; } +raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC) + : raw_fd_ostream(Filename, EC, sys::fs::CD_CreateAlways, sys::fs::FA_Write, + sys::fs::OF_None) {} + +raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC, + sys::fs::CreationDisposition Disp) + : raw_fd_ostream(Filename, EC, Disp, sys::fs::FA_Write, sys::fs::OF_None) {} + +raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC, + sys::fs::FileAccess Access) + : raw_fd_ostream(Filename, EC, sys::fs::CD_CreateAlways, Access, + sys::fs::OF_None) {} + +raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC, + sys::fs::OpenFlags Flags) + : raw_fd_ostream(Filename, EC, sys::fs::CD_CreateAlways, sys::fs::FA_Write, + Flags) {} + raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC, + sys::fs::CreationDisposition Disp, + sys::fs::FileAccess Access, sys::fs::OpenFlags Flags) - : raw_fd_ostream(getFD(Filename, EC, Flags), true) {} + : raw_fd_ostream(getFD(Filename, EC, Disp, Access, Flags), true) {} /// FD is the file descriptor that this writes to. If ShouldClose is true, this /// closes the file when the stream is destroyed. Index: llvm/tools/llvm-ar/llvm-ar.cpp =================================================================== --- llvm/tools/llvm-ar/llvm-ar.cpp +++ llvm/tools/llvm-ar/llvm-ar.cpp @@ -390,6 +390,7 @@ int FD; failIfError(sys::fs::openFileForWrite(sys::path::filename(Name), FD, + sys::fs::CD_CreateAlways, sys::fs::F_None, Mode), Name); Index: llvm/tools/llvm-cov/SourceCoverageView.cpp =================================================================== --- llvm/tools/llvm-cov/SourceCoverageView.cpp +++ llvm/tools/llvm-cov/SourceCoverageView.cpp @@ -65,7 +65,8 @@ return errorCodeToError(E); std::error_code E; - raw_ostream *RawStream = new raw_fd_ostream(FullPath, E, sys::fs::F_RW); + raw_ostream *RawStream = + new raw_fd_ostream(FullPath, E, sys::fs::FA_Read | sys::fs::FA_Write); auto OS = CoveragePrinter::OwnedStream(RawStream); if (E) return errorCodeToError(E); Index: llvm/tools/llvm-cov/TestingSupport.cpp =================================================================== --- llvm/tools/llvm-cov/TestingSupport.cpp +++ llvm/tools/llvm-cov/TestingSupport.cpp @@ -75,8 +75,7 @@ return 1; int FD; - if (auto Err = - sys::fs::openFileForWrite(OutputFilename, FD, sys::fs::F_None)) { + if (auto Err = sys::fs::openFileForWrite(OutputFilename, FD)) { errs() << "error: " << Err.message() << "\n"; return 1; } Index: llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp =================================================================== --- llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp +++ llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp @@ -200,7 +200,8 @@ } else { int ResultFD = 0; llvm::cantFail(llvm::errorCodeToError( - openFileForWrite(Filename, ResultFD, llvm::sys::fs::F_Text))); + openFileForWrite(Filename, ResultFD, llvm::sys::fs::CD_CreateAlways, + llvm::sys::fs::F_Text))); llvm::raw_fd_ostream Ostr(ResultFD, true /*shouldClose*/); writeYamlTo(Context, Ostr); } Index: llvm/tools/llvm-exegesis/llvm-exegesis.cpp =================================================================== --- llvm/tools/llvm-exegesis/llvm-exegesis.cpp +++ llvm/tools/llvm-exegesis/llvm-exegesis.cpp @@ -156,7 +156,8 @@ } std::error_code ErrorCode; llvm::raw_fd_ostream ClustersOS(OutputFilename, ErrorCode, - llvm::sys::fs::F_RW); + llvm::sys::fs::FA_Read | + llvm::sys::fs::FA_Write); if (ErrorCode) llvm::report_fatal_error("cannot open out file: " + OutputFilename); if (auto Err = Analyzer.run(ClustersOS)) Index: llvm/tools/llvm-rc/llvm-rc.cpp =================================================================== --- llvm/tools/llvm-rc/llvm-rc.cpp +++ llvm/tools/llvm-rc/llvm-rc.cpp @@ -171,8 +171,8 @@ "No more than one output file should be provided (using /FO flag)."); std::error_code EC; - auto FOut = - llvm::make_unique(OutArgsInfo[0], EC, sys::fs::F_RW); + auto FOut = llvm::make_unique( + OutArgsInfo[0], EC, sys::fs::FA_Read | sys::fs::FA_Write); if (EC) fatalError("Error opening output file '" + OutArgsInfo[0] + "': " + EC.message()); Index: llvm/unittests/Support/LockFileManagerTest.cpp =================================================================== --- llvm/unittests/Support/LockFileManagerTest.cpp +++ llvm/unittests/Support/LockFileManagerTest.cpp @@ -60,7 +60,7 @@ sys::path::append(TmpFileLock, "file.lock-000"); int FD; - EC = sys::fs::openFileForWrite(StringRef(TmpFileLock), FD, sys::fs::F_None); + EC = sys::fs::openFileForWrite(StringRef(TmpFileLock), FD); ASSERT_FALSE(EC); int Ret = close(FD); Index: llvm/unittests/Support/Path.cpp =================================================================== --- llvm/unittests/Support/Path.cpp +++ llvm/unittests/Support/Path.cpp @@ -49,8 +49,23 @@ } else { \ } +#define ASSERT_ERROR(x) \ + if (std::error_code ASSERT_NO_ERROR_ec = x) { \ + } else { \ + SmallString<128> MessageStorage; \ + raw_svector_ostream Message(MessageStorage); \ + Message << #x ": did not return a failure error code.\n"; \ + GTEST_FATAL_FAILURE_(MessageStorage.c_str()); \ + } + namespace { +struct FileDescriptorCloser { + explicit FileDescriptorCloser(int FD) : FD(FD) {} + ~FileDescriptorCloser() { ::close(FD); } + int FD; +}; + TEST(is_separator, Works) { EXPECT_TRUE(path::is_separator('/')); EXPECT_FALSE(path::is_separator('\0')); @@ -436,6 +451,7 @@ /// Unique temporary directory in which all created filesystem entities must /// be placed. It is removed at the end of each test (must be empty). SmallString<128> TestDirectory; + SmallString<128> NonExistantFile; void SetUp() override { ASSERT_NO_ERROR( @@ -443,6 +459,11 @@ // We don't care about this specific file. errs() << "Test Directory: " << TestDirectory << '\n'; errs().flush(); + NonExistantFile = TestDirectory; + + // Even though this value is hardcoded, is a 128-bit GUID, so we should be + // guaranteed that this file will never exist. + sys::path::append(NonExistantFile, "1B28B495C16344CB9822E588CD4C3EF0"); } void TearDown() override { ASSERT_NO_ERROR(fs::remove(TestDirectory.str())); } @@ -1219,8 +1240,8 @@ // Open the file for read int FileDescriptor2; SmallString<64> ResultPath; - ASSERT_NO_ERROR( - fs::openFileForRead(Twine(TempPath), FileDescriptor2, &ResultPath)) + ASSERT_NO_ERROR(fs::openFileForRead(Twine(TempPath), FileDescriptor2, + fs::OF_None, &ResultPath)) // If we succeeded, check that the paths are the same (modulo case): if (!ResultPath.empty()) { @@ -1235,6 +1256,209 @@ ::close(FileDescriptor); } +static void createFileWithData(const Twine &Path, bool ShouldExistBefore, + fs::CreationDisposition Disp, StringRef Data) { + int FD; + ASSERT_EQ(ShouldExistBefore, fs::exists(Path)); + ASSERT_NO_ERROR(fs::openFileForWrite(Path, FD, Disp)); + FileDescriptorCloser Closer(FD); + ASSERT_TRUE(fs::exists(Path)); + + ASSERT_EQ(Data.size(), write(FD, Data.data(), Data.size())); +} + +static void verifyFileContents(const Twine &Path, StringRef Contents) { + auto Buffer = MemoryBuffer::getFile(Path); + ASSERT_TRUE((bool)Buffer); + StringRef Data = Buffer.get()->getBuffer(); + ASSERT_EQ(Data, Contents); +} + +TEST_F(FileSystemTest, CreateNew) { + int FD; + Optional Closer; + + // Succeeds if the file does not exist. + ASSERT_FALSE(fs::exists(NonExistantFile)); + ASSERT_NO_ERROR(fs::openFileForWrite(NonExistantFile, FD, fs::CD_CreateNew)); + ASSERT_TRUE(fs::exists(NonExistantFile)); + + FileRemover Cleanup(NonExistantFile); + Closer.emplace(FD); + + // And creates a file of size 0. + sys::fs::file_status Status; + ASSERT_NO_ERROR(sys::fs::status(FD, Status)); + EXPECT_EQ(0ULL, Status.getSize()); + + // Close this first, before trying to re-open the file. + Closer.reset(); + + // But fails if the file does exist. + ASSERT_ERROR(fs::openFileForWrite(NonExistantFile, FD, fs::CD_CreateNew)); +} + +TEST_F(FileSystemTest, CreateAlways) { + int FD; + Optional Closer; + + // Succeeds if the file does not exist. + ASSERT_FALSE(fs::exists(NonExistantFile)); + ASSERT_NO_ERROR( + fs::openFileForWrite(NonExistantFile, FD, fs::CD_CreateAlways)); + + Closer.emplace(FD); + + ASSERT_TRUE(fs::exists(NonExistantFile)); + + FileRemover Cleanup(NonExistantFile); + + // And creates a file of size 0. + uint64_t FileSize; + ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize)); + ASSERT_EQ(0, FileSize); + + // If we write some data to it re-create it with CreateAlways, it succeeds and + // truncates to 0 bytes. + ASSERT_EQ(4, write(FD, "Test", 4)); + + Closer.reset(); + + ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize)); + ASSERT_EQ(4, FileSize); + + ASSERT_NO_ERROR( + fs::openFileForWrite(NonExistantFile, FD, fs::CD_CreateAlways)); + Closer.emplace(FD); + ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize)); + ASSERT_EQ(0, FileSize); +} + +TEST_F(FileSystemTest, OpenExisting) { + int FD; + + // Fails if the file does not exist. + ASSERT_FALSE(fs::exists(NonExistantFile)); + ASSERT_ERROR(fs::openFileForWrite(NonExistantFile, FD, fs::CD_OpenExisting)); + ASSERT_FALSE(fs::exists(NonExistantFile)); + + // Make a dummy file now so that we can try again when the file does exist. + createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "Fizz"); + FileRemover Cleanup(NonExistantFile); + uint64_t FileSize; + ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize)); + ASSERT_EQ(4, FileSize); + + // If we re-create it with different data, it overwrites rather than + // appending. + createFileWithData(NonExistantFile, true, fs::CD_OpenExisting, "Buzz"); + verifyFileContents(NonExistantFile, "Buzz"); +} + +TEST_F(FileSystemTest, OpenAlways) { + // Succeeds if the file does not exist. + createFileWithData(NonExistantFile, false, fs::CD_OpenAlways, "Fizz"); + FileRemover Cleanup(NonExistantFile); + uint64_t FileSize; + ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize)); + ASSERT_EQ(4, FileSize); + + // Now re-open it and write again, verifying the contents get over-written. + createFileWithData(NonExistantFile, true, fs::CD_OpenAlways, "Bu"); + verifyFileContents(NonExistantFile, "Buzz"); +} + +TEST_F(FileSystemTest, AppendSetsCorrectFileOffset) { + fs::CreationDisposition Disps[] = {fs::CD_CreateAlways, fs::CD_OpenAlways, + fs::CD_OpenExisting}; + + // Write some data and re-open it with every possible disposition (this is a + // hack that shouldn't work, but is left for compatibility. F_Append + // overrides + // the specified disposition. + for (fs::CreationDisposition Disp : Disps) { + int FD; + Optional Closer; + + createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "Fizz"); + + FileRemover Cleanup(NonExistantFile); + + uint64_t FileSize; + ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize)); + ASSERT_EQ(4, FileSize); + ASSERT_NO_ERROR( + fs::openFileForWrite(NonExistantFile, FD, Disp, fs::OF_Append)); + Closer.emplace(FD); + ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize)); + ASSERT_EQ(4U, FileSize); + + ASSERT_EQ(4, write(FD, "Buzz", 4)); + Closer.reset(); + + verifyFileContents(NonExistantFile, "FizzBuzz"); + } +} + +static void verifyRead(int FD, StringRef Data, bool ShouldSucceed) { + std::vector Buffer; + Buffer.resize(Data.size()); + int Result = ::read(FD, Buffer.data(), Buffer.size()); + if (ShouldSucceed) { + ASSERT_EQ(Result, Data.size()); + ASSERT_EQ(Data, StringRef(Buffer.data(), Buffer.size())); + } else { + ASSERT_EQ(-1, Result); + ASSERT_EQ(EBADF, errno); + } +} + +static void verifyWrite(int FD, StringRef Data, bool ShouldSucceed) { + int Result = ::write(FD, Data.data(), Data.size()); + if (ShouldSucceed) + ASSERT_EQ(Result, Data.size()); + else { + ASSERT_EQ(-1, Result); + ASSERT_EQ(EBADF, errno); + } +} + +TEST_F(FileSystemTest, ReadOnlyFileCantWrite) { + createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "Fizz"); + FileRemover Cleanup(NonExistantFile); + + int FD; + ASSERT_NO_ERROR(fs::openFileForRead(NonExistantFile, FD)); + FileDescriptorCloser Closer(FD); + + verifyWrite(FD, "Buzz", false); + verifyRead(FD, "Fizz", true); +} + +TEST_F(FileSystemTest, WriteOnlyFileCantRead) { + createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "Fizz"); + FileRemover Cleanup(NonExistantFile); + + int FD; + ASSERT_NO_ERROR( + fs::openFileForWrite(NonExistantFile, FD, fs::CD_OpenExisting)); + FileDescriptorCloser Closer(FD); + verifyRead(FD, "Fizz", false); + verifyWrite(FD, "Buzz", true); +} + +TEST_F(FileSystemTest, ReadWriteFileCanReadOrWrite) { + createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "Fizz"); + FileRemover Cleanup(NonExistantFile); + + int FD; + ASSERT_NO_ERROR(fs::openFileForReadWrite(NonExistantFile, FD, + fs::CD_OpenExisting, fs::OF_None)); + FileDescriptorCloser Closer(FD); + verifyRead(FD, "Fizz", true); + verifyWrite(FD, "Buzz", true); +} + TEST_F(FileSystemTest, set_current_path) { SmallString<128> path; Index: llvm/unittests/Support/ReplaceFileTest.cpp =================================================================== --- llvm/unittests/Support/ReplaceFileTest.cpp +++ llvm/unittests/Support/ReplaceFileTest.cpp @@ -31,7 +31,7 @@ std::error_code CreateFileWithContent(const SmallString<128> &FilePath, const StringRef &content) { int FD = 0; - if (std::error_code ec = fs::openFileForWrite(FilePath, FD, fs::F_None)) + if (std::error_code ec = fs::openFileForWrite(FilePath, FD)) return ec; const bool ShouldClose = true; Index: llvm/unittests/Support/raw_pwrite_stream_test.cpp =================================================================== --- llvm/unittests/Support/raw_pwrite_stream_test.cpp +++ llvm/unittests/Support/raw_pwrite_stream_test.cpp @@ -84,7 +84,7 @@ #ifdef LLVM_ON_UNIX TEST(raw_pwrite_ostreamTest, TestDevNull) { int FD; - sys::fs::openFileForWrite("/dev/null", FD, sys::fs::F_None); + sys::fs::openFileForWrite("/dev/null", FD, sys::fs::CD_OpenExisting); raw_fd_ostream OS(FD, true); OS << "abcd"; StringRef Test = "test";