[PATCH] D62718: [llvm-objcopy] Change handling of output file permissions

[Jordan Rupprecht via Phabricator via llvm-commits]

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

Just a couple nits, but I think this is ready to commit now. It can be updated whenever D62838 <https://reviews.llvm.org/D62838> lands. Thanks for all the iterations!



================
Comment at: llvm/tools/llvm-objcopy/Buffer.h:50
 
-  explicit FileBuffer(StringRef FileName) : Buffer(FileName) {}
+  explicit FileBuffer(StringRef FileName, unsigned Flags = 0)
+      : Buffer(FileName), Flags(Flags) {}
----------------
"explicit" can be removed now that this is two-args (and there's no implicit-conversion risk)


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:152
 
+static bool isExecutableObject(object::Binary &Binary) {
+  if (auto *ELFObj = dyn_cast<object::ELFObjectFileBase>(&Binary))
----------------
Can you add a TODO to use isExecutableObject from D62838 once that's submitted?


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:154
+  if (auto *ELFObj = dyn_cast<object::ELFObjectFileBase>(&Binary))
+    return ELFObj->getEType() & ELF::ET_EXEC;
+  else if (auto *COFFObj = dyn_cast<object::COFFObjectFile>(&Binary))
----------------
getEType() == ELF::ET_EXEC


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62718