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

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added inline comments.


================
Comment at: tools/llvm-objcopy/COFF/Object.h:83
+struct Section {
+  object::coff_section Header;
+  ArrayRef<uint8_t> Contents;
----------------
mstorsjo wrote:
> jakehehrlich wrote:
> > 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.
> So far, I don't keep a copy of the original header anywhere but I just patch things in this one copy (marginally when just doing a plain copy, patching more when actually changing things) before finally writing it to the output with one memcpy, so there's no distinction between original and current.
> 
> If we need to keep the original header separately later, couldn't we add the separate OriginalHeader field at that point?
I'd say if you ever have to keep a single "Original" field then you should keep the whole original header.


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

https://reviews.llvm.org/D54939