[PATCH] D16991: Split the creation of program headers in a few steps

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/Writer.cpp:1198-1200
@@ -1183,1 +1197,5 @@
+template <class ELFT> void Writer<ELFT>::scanHeaders() {
+  auto AddHdr = [this](unsigned Type, unsigned Flags) -> Phdr & {
+    return *Phdrs.emplace(Phdrs.end(), Type, Flags);
+  };
 
----------------
ruiu wrote:
> You may want to return a pointer instead of a reference because in a few places in this function you wrote "&AddHdr".
I agree here, that also eliminates the need of direct specify of return type for functor (it usually doesn't look good, here as well).

================
Comment at: ELF/Writer.cpp:1296-1302
@@ +1295,9 @@
+    const Phdr &P = Phdrs[I];
+    if (P.H.p_type == PT_GNU_RELRO) {
+      auto I = std::find(OutputSections.begin(), OutputSections.end(), P.Last);
+      ++I;
+      if (I != OutputSections.end() && needsPhdr(*I) &&
+          !((*I)->getFlags() & SHF_TLS))
+        PageAlign.insert(*I);
+    }
+    if (P.H.p_type != PT_LOAD)
----------------
ruiu wrote:
> Can you add a comment for this piece of code? It is not obvious.
If you're not using range-loop (but I think it would be ok here),
could you not use the variable name that replaces the name in global scope ? I mean you already have "I" above.


http://reviews.llvm.org/D16991