[PATCH] D15191: [ELF] Support PHDRS command

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 3 10:45:50 PST 2015


ruiu added a comment.

First round of review.


================
Comment at: ELF/LinkerScript.cpp:295-298
@@ +294,6 @@
+  while (!skip("}")) {
+    Config->Phdrs.emplace_back();
+    Phdr &PH = Config->Phdrs.back();
+    PH.Name = next();
+    PH.Type = readPhdrType();
+    expect(";");
----------------
  StringRef Name = next();
  uint32_t Type = readPhdrType();
  Config->Phdrs.emplace_back({Name, Type});

================
Comment at: ELF/LinkerScript.cpp:329
@@ +328,3 @@
+
+  // Read headers of the section.
+  while (true) {
----------------
It's better to be a bit more specific, say, read zero or more tokens in the form of :phdr.

================
Comment at: ELF/LinkerScript.cpp:330
@@ +329,3 @@
+  // Read headers of the section.
+  while (true) {
+    StringRef Tok = peek();
----------------
for (;;) for consistency.

================
Comment at: ELF/Writer.cpp:971
@@ +970,3 @@
+    }
+  } else {
+    SmallMapVector<StringRef, IndicesVector, 8> PhdrsToFill = PhdrNameToIndices;
----------------
This function is already too long and need to be refactored. Please at least factor this new code out from this function.


http://reviews.llvm.org/D15191





More information about the llvm-commits mailing list