jhenderson added inline comments.
================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:135
static std::unique_ptr<Writer> createELFWriter(const CopyConfig &Config,
+ const ELFCopyConfig &ELFConfig,
Object &Obj, Buffer &Buf,
----------------
For some warning levels, this will produce a warning as it is unused. Probably, you should comment out the name: `const ELFCopyConfig &/*ELFConfig*/`.
Of course, this becomes moot if you follow my suggestion of only passing in `ELFConfig`.
================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:172-173
static Expected<ArrayRef<uint8_t>>
-findBuildID(const CopyConfig &Config, const object::ELFFile<ELFT> &In) {
+findBuildID(const CopyConfig &Config, const ELFCopyConfig &ELFConfig,
+ const object::ELFFile<ELFT> &In) {
auto PhdrsOrErr = In.program_headers();
----------------
This and several other functions don't look like they need to change their signatures, since you don't use the ELF-specific parameters. Please revert them, or switch to passing in only `ELFConfig` and NOT `Config`.
================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.h:29-30
Buffer &Out);
-Error executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In,
- Buffer &Out);
+Error executeObjcopyOnRawBinary(const CopyConfig &Config,
+ const ELFCopyConfig &ELFConfig,
+ MemoryBuffer &In, Buffer &Out);
----------------
Since `ELFConfig` has `Config` as a member, you could just pass in `ELFConfig` in this and the other functions here, and use that.
================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:271
/// format-agnostic modifications, i.e. preserving dates.
-static Error executeObjcopy(const CopyConfig &Config) {
+static Error executeObjcopy(Configurations &Cfg) {
sys::fs::file_status Stat;
----------------
Here and elsewhere, I'd keep the name as `Config`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67139/new/
https://reviews.llvm.org/D67139