[PATCH] D24830: [ELF] - Linkerscript: implemented BYTE/SHORT/LONG/QUAD commands.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:424
@@ -417,1 +423,3 @@
   }
+  // It handles the bytes data command, which places some byte value into output
+  // section and moves the location counter in according to value data size.
----------------
Add a blank line before this.

Maybe just

  // Handle BYTE(), SHORT(), LONG(), or QUAD().

================
Comment at: ELF/LinkerScript.cpp:717
@@ +716,3 @@
+  int I = getSectionIndex(Name);
+  OutputSectionCommand *Cmd = dyn_cast_or_null<OutputSectionCommand>(
+      (I == INT_MAX) ? nullptr : Opt.Commands[I].get());
----------------
This looks odd. You can return early.

  if (I == INT_MAX)
    return;
  

================
Comment at: ELF/LinkerScript.cpp:726
@@ +725,3 @@
+      // endianness, the value will be stored in that endianness.
+      switch (DataCmd->Size) {
+      case 1:
----------------
I'd factor this switch out as an independent functions so that we can do something like this here.

  writeInt<ELFT>(Buf + DataCmd->Offset, DataCmd->Data, DataCmd->Size);

where writeInt is defined as

  writeInt(uint8_t *Buf, uint64_t Data, uint64_t Size);

================
Comment at: ELF/LinkerScript.cpp:1519
@@ +1518,3 @@
+BytesDataCommand *ScriptParser::readBytesDataCommand(StringRef Tok) {
+  unsigned Size = StringSwitch<unsigned>(Tok)
+                      .Case("BYTE", 1)
----------------
Use int instead of unsigned so to remove the cast to unsigned below.


https://reviews.llvm.org/D24830