[PATCH] D123821: [llvm-objcopy][NFC] refactor restoreStatOnFile out of llvm-objcopy.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Mostly looks fine to me. Just a couple of points/questions.



================
Comment at: llvm/include/llvm/Support/FileUtilities.h:114-115
+
+  /// FilePermissionsCarrier helps to copy permissions from the input file to
+  /// output one. It memorizes the status of the input file and can apply
+  /// permissions and dates to the output file.
----------------



================
Comment at: llvm/include/llvm/Support/FileUtilities.h:117
+  /// permissions and dates to the output file.
+  class FilePermissionsCarrier {
+  public:
----------------
I wonder if `FilePermssionsApplier` might make it clearer why this isn't just a POD type (and more importantly why it isn't just a copy of `sys::fs::perms`?


================
Comment at: llvm/lib/Support/FileUtilities.cpp:341
+
+Error FilePermissionsCarrier::apply(
+    StringRef OutputFilename, bool CopyDates,
----------------
Why do you need this extra layer here and not just do the check in the other `apply` function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123821