[PATCH] D97656: [llvm-objcopy] Initial XCOFF32 support.

[EsmeYi via Phabricator via llvm-commits]

Esme added inline comments.


================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:339
   int32_t getTimeStamp() const;
+  const XCOFFFileHeader32 *getFileHeader32() const;
 
----------------
shchenz wrote:
> I think it should be ok to call `fileHeader32` directly in the Reader?
`fileHeader32()` is a private member, so I added the public function `getFileHeader32()`.


================
Comment at: llvm/tools/llvm-objcopy/XCOFF/XCOFFObjcopy.cpp:39
+        llvm::errc::invalid_argument,
+        "no flags are supported yet, only basic copying is allowed");
+  }
----------------
shchenz wrote:
> From this comments, seems for now llvm-objcopy for XCOFF does not take any options? Even for `-h` or `-v`?
> If so, I think the options list here should not be complete. For example, we miss `StripDebug` and there are also many others.
> 
> Maybe we need a new method to tell whether an option is specified instead of listing all of them.
I have no idea about the new method, so I chose to list all of these options, just like what other targets have done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97656