[PATCH] D38283: Do not remove a target file in FileOutputBuffer::create().

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 26 15:31:21 PDT 2017


ruiu created this revision.
Herald added a subscriber: hiraditya.

FileOutputBuffer::create() attempts to remove a target file if the file
is a regular one, which results in an unexpected result in a failure
scenario.

If something goes wrong and the user of FileOutputBuffer decides to not
call commit(), it leaves nothing. An existing file was removed, and no
new file was created.

What we should do is to atomically replace an existing file with a new
file using rename(), so that it wouldn't remove an existing file without
creating a new one.


https://reviews.llvm.org/D38283

Files:
  llvm/lib/Support/FileOutputBuffer.cpp


Index: llvm/lib/Support/FileOutputBuffer.cpp
===================================================================
--- llvm/lib/Support/FileOutputBuffer.cpp
+++ llvm/lib/Support/FileOutputBuffer.cpp
@@ -65,13 +65,6 @@
       IsRegular = false;
   }
 
-  if (IsRegular) {
-    // Delete target file.
-    EC = sys::fs::remove(FilePath);
-    if (EC)
-      return EC;
-  }
-
   SmallString<128> TempFilePath;
   int FD;
   if (IsRegular) {
@@ -125,7 +118,7 @@
 
   std::error_code EC;
   if (IsRegular) {
-    // Rename file to final name.
+    // Atomically replace the existing file with the new one.
     EC = sys::fs::rename(Twine(TempPath), Twine(FinalPath));
     sys::DontRemoveFileOnSignal(TempPath);
   } else {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38283.116683.patch
Type: text/x-patch
Size: 718 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170926/98638584/attachment-0001.bin>


More information about the llvm-commits mailing list