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

[Zachary Turner via llvm-commits]

I don’t think this will replace an existing file will it? It looks like it
will fail if the file already exists
On Tue, Sep 26, 2017 at 3:31 PM Rui Ueyama via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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 --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170926/7ef621ed/attachment.html>