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

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

Thank you for the review.



================
Comment at: llvm/include/llvm/Support/FileUtilities.h:117
+  /// permissions and dates to the output file.
+  class FilePermissionsCarrier {
+  public:
----------------
jhenderson wrote:
> 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.
Ah, I see. thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123821