[PATCH] D67221: [yaml2obj] Write the section header table after section contents

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 5 05:55:17 PDT 2019


grimar added inline comments.


================
Comment at: lib/ObjectYAML/ELFEmitter.cpp:1060
+  Elf_Ehdr Header;
+  uint64_t SHOff;
+  CBA.getOSAndAlignedOffset(SHOff, sizeof(typename ELFT::uint));
----------------
Side note: we are not consistent in naming... we have `ShOffset` vs `SHOffset` and what is worse `IO.mapOptional("SHOffset", FileHdr.SHOffset);` vs `IO.mapOptional("ShOffset", Section.ShOffset)`


================
Comment at: lib/ObjectYAML/ELFEmitter.cpp:1063
+  State.initELFHeader(Header, SHOff);
   OS.write((const char *)&Header, sizeof(Header));
+
----------------
What do you think about moving this code to a new method which can replace the
`initELFHeader`? Something like:

```
void ELFState<ELFT>::writeElfHeader() {
  uint64_t SHOff;
  CBA.getOSAndAlignedOffset(SHOff, sizeof(typename ELFT::uint));

  Elf_Ehdr Header;

  /* Place the code from initELFHeader() method here */

  OS.write((const char *)&Header, sizeof(Header));
}
```

That way you do not need to declare and pass `Elf_Ehdr` and `SHOff` from here.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67221





More information about the llvm-commits mailing list