Add a flag to FileOutputBuffer that allows modification.

FileOutputBuffer creates a temp file and on commit atomically
renames the temp file to the destination file.  Sometimes we
want to modify an existing file in place, but still have the
atomicity guarantee.  To do this we can initialize the contents
of the temp file from the destination file (if it exists), that
way the resulting FileOutputBuffer can have only selective
bytes modified.  Committing will then atomically replace the
destination file as desired.

llvm-svn: 335902

include/llvm/Support/FileOutputBuffer.h

@@ -30,13 +30,25 @@ namespace llvm {
 /// not committed, the file will be deleted in the FileOutputBuffer destructor.
 class FileOutputBuffer {
 public:
-  enum  {
-    F_executable = 1  /// set the 'x' bit on the resulting file
+  enum {
+    /// set the 'x' bit on the resulting file
+    F_executable = 1,
+
+    /// the contents of the new file are initialized from the file that exists
+    /// at the location (if present).  This allows in-place modification of an
+    /// existing file.
+    F_modify = 2
   };
 
   /// Factory method to create an OutputBuffer object which manages a read/write
   /// buffer of the specified size. When committed, the buffer will be written
   /// to the file at the specified path.
+  ///
+  /// When F_modify is specified and \p FilePath refers to an existing on-disk
+  /// file \p Size may be set to -1, in which case the entire file is used.
+  /// Otherwise, the file shrinks or grows as necessary based on the value of
+  /// \p Size.  It is an error to specify F_modify and Size=-1 if \p FilePath
+  /// does not exist.
   static Expected<std::unique_ptr<FileOutputBuffer>>
   create(StringRef FilePath, size_t Size, unsigned Flags = 0);
 

include/llvm/Support/FileSystem.h

@@ -394,6 +394,12 @@ std::error_code rename(const Twine &from, const Twine &to);
 /// @param To The path to copy to. This is created.
 std::error_code copy_file(const Twine &From, const Twine &To);
 
+/// Copy the contents of \a From to \a To.
+///
+/// @param From The path to copy from.
+/// @param To The open file descriptor of the destinatino file.
+std::error_code copy_file(const Twine &From, int ToFD);
+
 /// Resize path to size. File is resized as if by POSIX truncate().
 ///
 /// @param FD Input file descriptor.

lib/Support/FileOutputBuffer.cpp

@@ -110,24 +110,30 @@ createInMemoryBuffer(StringRef Path, size_t Size, unsigned Mode) {
 }
 
 static Expected<std::unique_ptr<OnDiskBuffer>>
-createOnDiskBuffer(StringRef Path, size_t Size, unsigned Mode) {
+createOnDiskBuffer(StringRef Path, size_t Size, bool InitExisting,
+                   unsigned Mode) {
   Expected<fs::TempFile> FileOrErr =
       fs::TempFile::create(Path + ".tmp%%%%%%%", Mode);
   if (!FileOrErr)
     return FileOrErr.takeError();
   fs::TempFile File = std::move(*FileOrErr);
 
+  if (InitExisting) {
+    if (auto EC = sys::fs::copy_file(Path, File.FD))
+      return errorCodeToError(EC);
+  } else {
 #ifndef _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 function.
-  if (auto EC = fs::resize_file(File.FD, Size)) {
-    consumeError(File.discard());
-    return errorCodeToError(EC);
-  }
+    // 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 function.
+    if (auto EC = fs::resize_file(File.FD, Size)) {
+      consumeError(File.discard());
+      return errorCodeToError(EC);
+    }
 #endif
+  }
 
   // Mmap it.
   std::error_code EC;
@@ -151,6 +157,15 @@ FileOutputBuffer::create(StringRef Path, size_t Size, unsigned Flags) {
   fs::file_status Stat;
   fs::status(Path, Stat);
 
+  if ((Flags & F_modify) && Size == size_t(-1)) {
+    if (Stat.type() == fs::file_type::regular_file)
+      Size = Stat.getSize();
+    else if (Stat.type() == fs::file_type::file_not_found)
+      return errorCodeToError(errc::no_such_file_or_directory);
+    else
+      return errorCodeToError(errc::invalid_argument);
+  }
+
   // Usually, we want to create OnDiskBuffer to create a temporary file in
   // the same directory as the destination file and atomically replaces it
   // by rename(2).
@@ -165,7 +180,7 @@ FileOutputBuffer::create(StringRef Path, size_t Size, unsigned Flags) {
   case fs::file_type::regular_file:
   case fs::file_type::file_not_found:
   case fs::file_type::status_error:
-    return createOnDiskBuffer(Path, Size, Mode);
+    return createOnDiskBuffer(Path, Size, !!(Flags & F_modify), Mode);
   default:
     return createInMemoryBuffer(Path, Size, Mode);
   }

lib/Support/Path.cpp

@@ -927,16 +927,7 @@ std::error_code create_directories(const Twine &Path, bool IgnoreExisting,
   return create_directory(P, IgnoreExisting, Perms);
 }
 
-std::error_code copy_file(const Twine &From, const Twine &To) {
-  int ReadFD, WriteFD;
-  if (std::error_code EC = openFileForRead(From, ReadFD, OF_None))
-    return EC;
-  if (std::error_code EC =
-          openFileForWrite(To, WriteFD, CD_CreateAlways, OF_None)) {
-    close(ReadFD);
-    return EC;
-  }
-
+static std::error_code copy_file_internal(int ReadFD, int WriteFD) {
   const size_t BufSize = 4096;
   char *Buf = new char[BufSize];
   int BytesRead = 0, BytesWritten = 0;
@@ -953,8 +944,6 @@ std::error_code copy_file(const Twine &From, const Twine &To) {
     if (BytesWritten < 0)
       break;
   }
-  close(ReadFD);
-  close(WriteFD);
   delete[] Buf;
 
   if (BytesRead < 0 || BytesWritten < 0)
@@ -962,6 +951,36 @@ std::error_code copy_file(const Twine &From, const Twine &To) {
   return std::error_code();
 }
 
+std::error_code copy_file(const Twine &From, const Twine &To) {
+  int ReadFD, WriteFD;
+  if (std::error_code EC = openFileForRead(From, ReadFD, OF_None))
+    return EC;
+  if (std::error_code EC =
+          openFileForWrite(To, WriteFD, CD_CreateAlways, OF_None)) {
+    close(ReadFD);
+    return EC;
+  }
+
+  std::error_code EC = copy_file_internal(ReadFD, WriteFD);
+
+  close(ReadFD);
+  close(WriteFD);
+
+  return EC;
+}
+
+std::error_code copy_file(const Twine &From, int ToFD) {
+  int ReadFD;
+  if (std::error_code EC = openFileForRead(From, ReadFD, OF_None))
+    return EC;
+
+  std::error_code EC = copy_file_internal(ReadFD, ToFD);
+
+  close(ReadFD);
+
+  return EC;
+}
+
 ErrorOr<MD5::MD5Result> md5_contents(int FD) {
   MD5 Hash;
 

unittests/Support/FileOutputBufferTest.cpp

@@ -11,6 +11,7 @@
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
@@ -121,4 +122,53 @@ TEST(FileOutputBuffer, Test) {
   // Clean up.
   ASSERT_NO_ERROR(fs::remove(TestDirectory.str()));
 }
+
+TEST(FileOutputBuffer, TestModify) {
+  // Create unique temporary directory for these tests
+  SmallString<128> TestDirectory;
+  {
+    ASSERT_NO_ERROR(
+      fs::createUniqueDirectory("FileOutputBuffer-modify", TestDirectory));
+  }
+
+  SmallString<128> File1(TestDirectory);
+  File1.append("/file");
+  // First write some data.
+  {
+    Expected<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
+      FileOutputBuffer::create(File1, 10);
+    ASSERT_NO_ERROR(errorToErrorCode(BufferOrErr.takeError()));
+    std::unique_ptr<FileOutputBuffer> &Buffer = *BufferOrErr;
+    memcpy(Buffer->getBufferStart(), "AAAAAAAAAA", 10);
+    ASSERT_NO_ERROR(errorToErrorCode(Buffer->commit()));
+  }
+
+  // Then re-open the file for modify and change only some bytes.
+  {
+    Expected<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
+        FileOutputBuffer::create(File1, size_t(-1), FileOutputBuffer::F_modify);
+    ASSERT_NO_ERROR(errorToErrorCode(BufferOrErr.takeError()));
+    std::unique_ptr<FileOutputBuffer> &Buffer = *BufferOrErr;
+    ASSERT_EQ(10, Buffer->getBufferSize());
+    uint8_t *Data = Buffer->getBufferStart();
+    Data[0] = 'X';
+    Data[9] = 'X';
+    ASSERT_NO_ERROR(errorToErrorCode(Buffer->commit()));
+  }
+
+  // Finally, re-open the file for read and verify that it has the modified
+  // contents.
+  {
+    ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr = MemoryBuffer::getFile(File1);
+    ASSERT_NO_ERROR(BufferOrErr.getError());
+    std::unique_ptr<MemoryBuffer> Buffer = std::move(*BufferOrErr);
+    ASSERT_EQ(10, Buffer->getBufferSize());
+    EXPECT_EQ(StringRef("XAAAAAAAAX"), Buffer->getBuffer());
+  }
+
+  // Clean up.
+  ASSERT_NO_ERROR(fs::remove(File1));
+  ASSERT_NO_ERROR(fs::remove(TestDirectory));
+}
+
 } // anonymous namespace