[PATCH] D54939: [RFC] [llvm-objcopy] Initial COFF support

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added a comment.

On the whole ELFObject vs Object thing. I'm in favor of just using Object but I don't really care too much. Having both the namespace and the name prefix is kind of verbose. A while back we had two kinds of Objects instead of just one but this was determined overtime to be a mistake for the ELF backend. If you want to do conversions between formats, even if the format is "binary" you'll probably want to avoid having two kinds of Object or if you do make sure you provide a sufficiently rich base interface to them to allow most code to be written in terms of that.



================
Comment at: tools/llvm-objcopy/COFF/Object.h:83
+struct Section {
+  object::coff_section Header;
+  ArrayRef<uint8_t> Contents;
----------------
It might be useful to declare this as the OriginalHeader and have a second Header. We discovered overtime that this would have been a good structure in the ELF case but currently we have a horrible hodgepodge of fields prefixed with "Original" and some without. Also, the name "Contents" doesn't necessarily need to change but it should be clear that such a thing is the original contents and nothing more.


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

https://reviews.llvm.org/D54939