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

[Jian Cai via Phabricator via llvm-commits]

jcai19 added inline comments.


================
Comment at: llvm/lib/Support/FileOutputBuffer.cpp:208
     else
-      return createOnDiskBuffer(Path, Size, Mode);
+      return createOnDiskBuffer(Path, Size, Mode, Flags & F_keep_ownership);
   default:
----------------
jcai19 wrote:
> rupprecht wrote:
> > 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.
> We could definitely delay the and operation into createOnDiskBuffer, but it is not a member function so we would have to use something like Flags & FileOutputBuffer::F_keep_ownership. I feel with the current implementation we could avoid spilling implementation details of FileOutputBuffer into createOnDiskBuffer. WDTY?
To further clarify, I meant we could create less dependency between FileOutputBuffer and createOnDiskBuffer with the current implementation so I feel it could be marginally better. Also correction to my typo, WDTY -> WDYT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93881