[PATCH] D40094: LTO: clarify why we need to gracefully handle sys::fs::rename failures

[Bob Haarman via Phabricator via llvm-commits]

inglorion updated this revision to Diff 123101.
inglorion added a comment.

@pcc, better?


https://reviews.llvm.org/D40094

Files:
  llvm/lib/LTO/Caching.cpp


Index: llvm/lib/LTO/Caching.cpp
===================================================================
--- llvm/lib/LTO/Caching.cpp
+++ llvm/lib/LTO/Caching.cpp
@@ -71,13 +71,15 @@
                              TempFilename + ": " +
                              MBOrErr.getError().message() + "\n");
 
-        // This is atomic on POSIX systems.
-        // On Windows, it can fail with permission denied if the destination
-        // file already exists. Since the existing file should be semantically
-        // equivalent to the one we are trying to write, we give AddBuffer
-        // a copy of the bytes we wrote in that case. We do this instead of
-        // just using the existing file, because the pruner might delete the
-        // file before we get a chance to use it.
+        // On POSIX systems, rename will atomically replace the destination if
+        // it already exists. We try to emulate this on Windows, but this may
+        // fail with a permission denied error (for example, if the destination
+        // is currently opened by another process that does not give us the
+        // sharing permissions we need). Since the existing file should be
+        // semantically equivalent to the one we are trying to write, we give
+        // AddBuffer a copy of the bytes we wrote in that case. We do this
+        // instead of just using the existing file, because the pruner might
+        // delete the file before we get a chance to use it.
         auto EC = sys::fs::rename(TempFilename, EntryPath);
         if (EC == errc::permission_denied) {
           auto MBCopy = MemoryBuffer::getMemBufferCopy(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40094.123101.patch
Type: text/x-patch
Size: 1637 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171115/58c5e0ac/attachment-0001.bin>