[PATCH] D93881: [llvm-objcopy] preserve file ownership when overwritten

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: llvm/lib/Support/FileOutputBuffer.cpp:140
+    fs::file_status Stat;
+    std::error_code EC = fs::status(Path, Stat);
+    if (!EC)
----------------
WDYT about doing a stat on the newly-generated tempfile so we can skip the redundant fchown if possible? (i.e. the common case of running as a non-root user)


================
Comment at: llvm/lib/Support/FileOutputBuffer.cpp:208
     else
-      return createOnDiskBuffer(Path, Size, Mode);
+      return createOnDiskBuffer(Path, Size, Mode, Flags & F_keep_ownership);
   default:
----------------
Personally, I was a little confused by `Flags & F_keep_ownership`, and I think it would be simpler to pass in the `unsigned Flags` param itself instead of `bool KeepOwnership`, and deferring the `Flags & F_keep_ownership` check until later.


================
Comment at: llvm/lib/Support/Unix/Path.inc:1215
+std::error_code changeFileOwnership(int FD, uint32_t Owner, uint32_t Group) {
+  auto Chown = [&]() { return ::fchown(FD, Owner, Group); };
+  // Retry if fchown call fails due to interruption.
----------------
`auto FChown`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93881