[PATCH] D81803: [Support] PR42623: Avoid setting the delete-on-close bit if a TempFile doesn't reside on a local drive

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added a comment.

It would be nice to retain Windows 7 support for now, seeing the number of users affected by this issue.



================
Comment at: llvm/lib/Support/Windows/Path.inc:407
+  // set DeleteFile to true, since it prevents opening the file for writes.
+  SmallVector<wchar_t, 128> FinalPath;
+  if (std::error_code EC = realPathFromHandle(Handle, FinalPath))
----------------
The error occurs on a second call with `setDeleteDisposition(..., false)` initiated by `TempFile::keep()`. A simple repro from https://bugs.llvm.org/show_bug.cgi?id=48378#c0 can be found here: https://pastebin.com/v51m3uBU

We could enclose this new block of code into `if (Delete) { ... }`. eg.
```
static std::error_code setDeleteDisposition(HANDLE Handle, bool Delete) {
  if (Delete) {
    // First, check if the file is on a network (non-local) drive. If so, don't
    // set DeleteFile to true, since it prevents opening the file for writes.
    SmallVector<wchar_t, 128> FinalPath;
    ...
    // At this point, the file is on a local drive.
  }

  FILE_DISPOSITION_INFO Disposition;
  Disposition.DeleteFile = Delete;
  if (!SetFileInformationByHandle(Handle, FileDispositionInfo, &Disposition,
                                  sizeof(Disposition)))
    return mapWindowsError(::GetLastError());
  return std::error_code();
}

```
The original report from @rdwampler suggests that the problem is not `SetFileInformationByHandle`, but writes that happen later in the process. In this particular case, the writes already happened.

WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81803/new/

https://reviews.llvm.org/D81803