[PATCH] D18743: [ELF] - Improved consept of dummies sections.

[Rui Ueyama via llvm-commits]

ruiu added a comment.

Generally looks good. A few nits.


================
Comment at: ELF/Writer.cpp:103
@@ -105,10 +102,3 @@
   std::vector<std::unique_ptr<OutputSectionBase<ELFT>>> OwningSections;
-
-  // We create a section for the ELF header and one for the program headers.
-  ArrayRef<OutputSectionBase<ELFT> *> getSections() const {
-    return makeArrayRef(OutputSections).slice(dummySectionsNum());
-  }
-  unsigned getNumSections() const {
-    return OutputSections.size() + 1 - dummySectionsNum();
-  }
+  unsigned getNumSections() const { return OutputSections.size() + 1; }
 
----------------
Why +1?

================
Comment at: ELF/Writer.cpp:1346
@@ -1357,1 +1345,3 @@
 
+template <class ELFT> void Writer<ELFT>::fixHeaders() {
+  uintX_t Off = 0;
----------------
This function needs a comment.

================
Comment at: ELF/Writer.cpp:1347-1349
@@ +1346,5 @@
+template <class ELFT> void Writer<ELFT>::fixHeaders() {
+  uintX_t Off = 0;
+  Out<ELFT>::ElfHeader->setVA(Off + Target->getVAStart());
+  Out<ELFT>::ElfHeader->setFileOffset(Off);
+  Off += Out<ELFT>::ElfHeader->getSize();
----------------
"Off" is always zero, so please do this.

  Out<ELFT>::ElfHeader->setVA(Target->getVAStart());
  Out<ELFT>::ElfHeader->setFileOffset(0);


http://reviews.llvm.org/D18743