Create a TempFile class.

This just adds a TempFile class and replaces the use in
FileOutputBuffer with it.

The only difference for now is better error handling. Followup work includes:

- Convert other user of temporary files to it.
- Add support for automatically deleting on windows.
- Add a createUnnamed method that returns a potentially unnamed
  file. It would be actually unnamed on modern linux and have a
  unknown name on windows.

llvm-svn: 318069

include/llvm/Support/FileSystem.h

@@ -31,6 +31,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Chrono.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MD5.h"
@@ -694,6 +695,40 @@ std::error_code createUniqueFile(const Twine &Model, int &ResultFD,
 std::error_code createUniqueFile(const Twine &Model,
                                  SmallVectorImpl<char> &ResultPath);
 
+/// Represents a temporary file.
+///
+/// The temporary file must be eventually discarded or given a final name and
+/// kept.
+///
+/// The destructor doesn't implicitly discard because there is no way to
+/// properly handle errors in a destructor.
+class TempFile {
+  bool Done = false;
+  TempFile(StringRef Name, int FD);
+
+public:
+  /// This creates a temporary file with createUniqueFile and schedules it for
+  /// deletion with sys::RemoveFileOnSignal.
+  static Expected<TempFile> create(const Twine &Model,
+                                   unsigned Mode = all_read | all_write);
+  TempFile(TempFile &&Other);
+
+  // Name of the temporary file.
+  std::string TmpName;
+
+  // The open file descriptor.
+  int FD = -1;
+
+  // Keep this with the given name.
+  Error keep(const Twine &Name);
+
+  // Delete the file.
+  Error discard();
+
+  // This checks that keep or delete was called.
+  ~TempFile();
+};
+
 /// @brief Create a file in the system temporary directory.
 ///
 /// The filename is of the form prefix-random_chars.suffix. Since the directory

lib/Support/FileOutputBuffer.cpp

@@ -17,7 +17,6 @@
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Memory.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/Signals.h"
 #include <system_error>
 
 #if !defined(_MSC_VER) && !defined(__MINGW32__)
@@ -34,9 +33,9 @@ using namespace llvm::sys;
 // with the temporary file on commit().
 class OnDiskBuffer : public FileOutputBuffer {
 public:
-  OnDiskBuffer(StringRef Path, StringRef TempPath,
+  OnDiskBuffer(StringRef Path, fs::TempFile Temp,
                std::unique_ptr<fs::mapped_file_region> Buf)
-      : FileOutputBuffer(Path), Buffer(std::move(Buf)), TempPath(TempPath) {}
+      : FileOutputBuffer(Path), Buffer(std::move(Buf)), Temp(std::move(Temp)) {}
 
   uint8_t *getBufferStart() const override { return (uint8_t *)Buffer->data(); }
 
@@ -51,21 +50,19 @@ public:
     Buffer.reset();
 
     // Atomically replace the existing file with the new one.
-    auto EC = fs::rename(TempPath, FinalPath);
-    sys::DontRemoveFileOnSignal(TempPath);
-    return errorCodeToError(EC);
+    return Temp.keep(FinalPath);
   }
 
   ~OnDiskBuffer() override {
     // Close the mapping before deleting the temp file, so that the removal
     // succeeds.
     Buffer.reset();
-    fs::remove(TempPath);
+    consumeError(Temp.discard());
   }
 
 private:
   std::unique_ptr<fs::mapped_file_region> Buffer;
-  std::string TempPath;
+  fs::TempFile Temp;
 };
 
 // A FileOutputBuffer which keeps data in memory and writes to the final
@@ -110,13 +107,11 @@ createInMemoryBuffer(StringRef Path, size_t Size, unsigned Mode) {
 
 static Expected<std::unique_ptr<OnDiskBuffer>>
 createOnDiskBuffer(StringRef Path, size_t Size, unsigned Mode) {
-  // Create new file in same directory but with random name.
-  SmallString<128> TempPath;
-  int FD;
-  if (auto EC = fs::createUniqueFile(Path + ".tmp%%%%%%%", FD, TempPath, Mode))
-    return errorCodeToError(EC);
-
-  sys::RemoveFileOnSignal(TempPath);
+  Expected<fs::TempFile> FileOrErr =
+      fs::TempFile::create(Path + ".tmp%%%%%%%", Mode);
+  if (!FileOrErr)
+    return FileOrErr.takeError();
+  fs::TempFile File = std::move(*FileOrErr);
 
 #ifndef LLVM_ON_WIN32
   // On Windows, CreateFileMapping (the mmap function on Windows)
@@ -124,18 +119,22 @@ createOnDiskBuffer(StringRef Path, size_t Size, unsigned Mode) {
   // extend the file beforehand. _chsize (ftruncate on Windows) is
   // pretty slow just like it writes specified amount of bytes,
   // so we should avoid calling that function.
-  if (auto EC = fs::resize_file(FD, Size))
+  if (auto EC = fs::resize_file(File.FD, Size)) {
+    consumeError(File.discard());
     return errorCodeToError(EC);
+  }
 #endif
 
   // Mmap it.
   std::error_code EC;
   auto MappedFile = llvm::make_unique<fs::mapped_file_region>(
-      FD, fs::mapped_file_region::readwrite, Size, 0, EC);
-  close(FD);
-  if (EC)
+      File.FD, fs::mapped_file_region::readwrite, Size, 0, EC);
+  if (EC) {
+    consumeError(File.discard());
     return errorCodeToError(EC);
-  return llvm::make_unique<OnDiskBuffer>(Path, TempPath, std::move(MappedFile));
+  }
+  return llvm::make_unique<OnDiskBuffer>(Path, std::move(File),
+                                         std::move(MappedFile));
 }
 
 // Create an instance of FileOutputBuffer.

lib/Support/Path.cpp

@@ -18,6 +18,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/Signals.h"
 #include <cctype>
 #include <cstring>
 
@@ -759,6 +760,58 @@ std::error_code createUniqueFile(const Twine &Model,
   return createUniqueEntity(Model, Dummy, ResultPath, false, 0, FS_Name);
 }
 
+TempFile::TempFile(StringRef Name, int FD) : TmpName(Name), FD(FD) {}
+TempFile::TempFile(TempFile &&Other) {
+  TmpName = std::move(Other.TmpName);
+  FD = Other.FD;
+  Other.Done = true;
+}
+
+TempFile::~TempFile() { assert(Done); }
+
+Error TempFile::discard() {
+  if (Done)
+    return Error::success();
+  Done = true;
+  // Always try to close and remove.
+  std::error_code RemoveEC = fs::remove(TmpName);
+  sys::DontRemoveFileOnSignal(TmpName);
+  if (close(FD) == -1) {
+    std::error_code EC = std::error_code(errno, std::generic_category());
+    return errorCodeToError(EC);
+  }
+  return errorCodeToError(RemoveEC);
+}
+
+Error TempFile::keep(const Twine &Name) {
+  assert(!Done);
+  Done = true;
+  // Always try to close and rename.
+  std::error_code RenameEC = fs::rename(TmpName, Name);
+  sys::DontRemoveFileOnSignal(TmpName);
+  if (close(FD) == -1) {
+    std::error_code EC(errno, std::generic_category());
+    return errorCodeToError(EC);
+  }
+  return errorCodeToError(RenameEC);
+}
+
+Expected<TempFile> TempFile::create(const Twine &Model, unsigned Mode) {
+  int FD;
+  SmallString<128> ResultPath;
+  if (std::error_code EC = createUniqueFile(Model, FD, ResultPath, Mode))
+    return errorCodeToError(EC);
+
+  // Make sure we delete the file when RemoveFileOnSignal fails.
+  TempFile Ret(ResultPath, FD);
+  if (sys::RemoveFileOnSignal(ResultPath)) {
+    consumeError(Ret.discard());
+    std::error_code EC(errc::operation_not_permitted);
+    return errorCodeToError(EC);
+  }
+  return std::move(Ret);
+}
+
 static std::error_code
 createTemporaryFile(const Twine &Model, int &ResultFD,
                     llvm::SmallVectorImpl<char> &ResultPath, FSEntity Type) {