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

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, once the name typo has been fixed.



================
Comment at: llvm/include/llvm/Support/FileUtilities.h:117
+  /// permissions and dates to the output file.
+  class FilePermissionsCarrier {
+  public:
----------------
avl wrote:
> jhenderson wrote:
> > 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`?
> sorry, did not get the second part of question. The FilePermissionsCarrier/FilePermssionsApplier already contains a copy of permissions:
> 
> 
> ```
>     sys::fs::file_status InputStatus;
> 
> ```
> What do you suggest to change?
Sorry, realised I made a couple of typos in my original comment.

`FilePermssionsApplier` -> `FilePermissionsApplier`

The name change was the only thing I was requesting. The rest of the comment was about why I recommended it:
# It's not a POD type but "Carrier" implies it just stores some data.
# The class is needed because it also does something to the data it is storing. We can't just use `sys::fs::perms` or similar directly because of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123821