[PATCH] D69146: [install-name-tool] Add first bits for install-name-tool

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap marked 2 inline comments as done.
alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:715-717
+// ParseInstallNameToolOptions returns the config and sets the input arguments.
+// If a help flag is set then ParseInstallNameToolOptions will print the help
+// messege and exit.
----------------
jhenderson wrote:
> Do we need this comment both in the header and in the .cpp?
the other parse* functions have it in the both places as well ...


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.h:75
   // copied-over without digging into its structure.
-  ArrayRef<uint8_t> Payload;
+  std::vector<uint8_t> Payload; 
 
----------------
rupprecht wrote:
> Changing this to an owning container could make llvm-objcopy/strip significantly slower on MachO files due to an extra copy. Is there any way to avoid this? Maybe even using a bump allocator (like StringSaver) if you do need to allocate in some cases?
This the payload of the load command, not the content of a sections etc. We can switch back to ArrayRef, but usually the total size is quite small (a few KBs in total), so I somehow preferred a simpler approach / less code. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D69146