Support outputting to /dev/null

[• rafael]

Summary

This is an alternative that involves a smaller code difference for regular and non regular files.

[ruiu]

lib/Support/FileOutputBuffer.cpp

62–63

Remove else.

72–73

Update the comment.

94

You want to call this only when IsRegular.

[• rafael]

Address comments.

[• rafael]

Delete temp file.

[ruiu]

LGTM with this fix.

lib/Support/FileOutputBuffer.cpp

128

I think I now understand why we had this code in the first place, and I believe we want to call this for both IsRegular and !IsRegular.

Once we are done with a temporary file, we rename or remove the temporary file, so the path will become nonexistent. After that, we don't want to try to remove that path, because it is useless, and sometimes harmful, if other process happens to use the same temporary filename.

[• espindola]

291485

include/llvm/Support/FileOutputBuffer.h

public:
   /// deallocates the buffer and the target file is never written.
   ~FileOutputBuffer();
 
 private:
   FileOutputBuffer(const FileOutputBuffer &) = delete;
   FileOutputBuffer &operator=(const FileOutputBuffer &) = delete;
 
   FileOutputBuffer(std::unique_ptr<llvm::sys::fs::mapped_file_region> R,
-                   StringRef Path, StringRef TempPath);
+                   StringRef Path, StringRef TempPath, bool IsRegular);
 
   std::unique_ptr<llvm::sys::fs::mapped_file_region> Region;
   SmallString<128>    FinalPath;
   SmallString<128>    TempPath;
+  bool IsRegular;
 };
 } // end namespace llvm
 
 #endif

lib/Support/FileOutputBuffer.cpp

 // Utility for creating a in-memory buffer that will be written to a file.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Errc.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Signals.h"
 #include <system_error>
 
 #if !defined(_MSC_VER) && !defined(__MINGW32__)
 #include <unistd.h>
 #else
 #include <io.h>
 #endif
 
 using llvm::sys::fs::mapped_file_region;
 
 namespace llvm {
 FileOutputBuffer::FileOutputBuffer(std::unique_ptr<mapped_file_region> R,
-                                   StringRef Path, StringRef TmpPath)
-    : Region(std::move(R)), FinalPath(Path), TempPath(TmpPath) {}
+                                   StringRef Path, StringRef TmpPath,
+                                   bool IsRegular)
+    : Region(std::move(R)), FinalPath(Path), TempPath(TmpPath),
+      IsRegular(IsRegular) {}
 
 FileOutputBuffer::~FileOutputBuffer() {
   // Close the mapping before deleting the temp file, so that the removal
   // succeeds.
   Region.reset();
   sys::fs::remove(Twine(TempPath));
 }
 
 ErrorOr<std::unique_ptr<FileOutputBuffer>>
 FileOutputBuffer::create(StringRef FilePath, size_t Size, unsigned Flags) {
-  // If file already exists, it must be a regular file (to be mappable).
+  // Check file is not a regular file, in which case we cannot remove it.
   sys::fs::file_status Stat;
   std::error_code EC = sys::fs::status(FilePath, Stat);
+  bool IsRegular = true;
   switch (Stat.type()) {
     case sys::fs::file_type::file_not_found:
       // If file does not exist, we'll create one.
       break;
     case sys::fs::file_type::regular_file: {
         // If file is not currently writable, error out.
         // FIXME: There is no sys::fs:: api for checking this.
         // FIXME: In posix, you use the access() call to check this.
       }
       break;
     default:
       if (EC)
         return EC;
-      else
-        return make_error_code(errc::operation_not_permitted);
+      IsRegular = false;
   }
 
+  if (IsRegular) {
     // Delete target file.
     EC = sys::fs::remove(FilePath);
     if (EC)
       return EC;
+  }
 
+  SmallString<128> TempFilePath;
+  int FD;
+  if (IsRegular) {
     unsigned Mode = sys::fs::all_read | sys::fs::all_write;
     // If requested, make the output file executable.
     if (Flags & F_executable)
       Mode |= sys::fs::all_exe;
-
     // Create new file in same directory but with random name.
-  SmallString<128> TempFilePath;
-  int FD;
     EC = sys::fs::createUniqueFile(Twine(FilePath) + ".tmp%%%%%%%", FD,
                                    TempFilePath, Mode);
+  } else {
+    // Create a temporary file. Since this is a special file, we will not move
+    // it and the new file can be in another filesystem. This avoids trying to
+    // create a temporary file in /dev when outputting to /dev/null for example.
+    EC = sys::fs::createTemporaryFile(sys::path::filename(FilePath), "", FD,
+                                      TempFilePath);
+  }
+
   if (EC)
     return EC;
 
   sys::RemoveFileOnSignal(TempFilePath);
 
 #ifndef LLVM_ON_WIN32
   // On Windows, CreateFileMapping (the mmap function on Windows)
   // automatically extends the underlying file. We don't need to
   // 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.
   EC = sys::fs::resize_file(FD, Size);
   if (EC)
     return EC;
 #endif
 
   auto MappedFile = llvm::make_unique<mapped_file_region>(
       FD, mapped_file_region::readwrite, Size, 0, EC);
   int Ret = close(FD);
   if (EC)
     return EC;
   if (Ret)
     return std::error_code(errno, std::generic_category());
 
-  std::unique_ptr<FileOutputBuffer> Buf(
-      new FileOutputBuffer(std::move(MappedFile), FilePath, TempFilePath));
+  std::unique_ptr<FileOutputBuffer> Buf(new FileOutputBuffer(
+      std::move(MappedFile), FilePath, TempFilePath, IsRegular));
   return std::move(Buf);
 }
 
 std::error_code FileOutputBuffer::commit() {
   // Unmap buffer, letting OS flush dirty pages to file on disk.
   Region.reset();
 
-
+  std::error_code EC;
+  if (IsRegular) {
     // Rename file to final name.
-  std::error_code EC = sys::fs::rename(Twine(TempPath), Twine(FinalPath));
+    EC = sys::fs::rename(Twine(TempPath), Twine(FinalPath));
     sys::DontRemoveFileOnSignal(TempPath);
+  } else {
+    EC = sys::fs::copy_file(TempPath, FinalPath);
+    std::error_code RMEC = sys::fs::remove(TempPath);
+    if (EC)
+      return EC;
+    return RMEC;
+  }
   return EC;
 }
 } // namespace