[PATCH] D17269: [ELF] - Implemented linkerscript sections padding.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 25 09:25:37 PST 2016


ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:69-70
@@ +68,4 @@
+  auto E = OutSections.end();
+  auto I = std::find_if(OutSections.begin(), E,
+                        [&](OutSection &C) { return C.Name == A; });
+  auto J = std::find_if(OutSections.begin(), E,
----------------
This new code with lambdas looks good to me.

================
Comment at: ELF/LinkerScript.cpp:417
@@ -396,1 +416,3 @@
 
+std::vector<uint8_t> ScriptParser::readHex(StringRef S) {
+  std::vector<uint8_t> Hex;
----------------
Let's rename parseHex. (Because readSomething usually reads Something from the current token stream.)

================
Comment at: ELF/LinkerScript.cpp:419
@@ +418,3 @@
+  std::vector<uint8_t> Hex;
+  Hex.reserve((S.size() + 1) / 2);
+
----------------
I don't think you need to call reserve. We are handling tiny data, so allocation cost should be negligible.

================
Comment at: ELF/LinkerScript.cpp:423
@@ +422,3 @@
+    StringRef B = S.substr(0, 2);
+    S = S.slice(2, S.size());
+    uint8_t H;
----------------
slice(2, S.size()) -> substr(2)

================
Comment at: ELF/LinkerScript.cpp:460-461
@@ +459,4 @@
+    OutSec.Filler = readHex(Tok);
+    if (Error)
+      return;
+    next();
----------------
Do you need to add check here? You can just go proceed.


http://reviews.llvm.org/D17269





More information about the llvm-commits mailing list