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

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 26 01:29:42 PST 2016


grimar added inline comments.

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

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

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

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


http://reviews.llvm.org/D17269





More information about the llvm-commits mailing list