[PATCH] D24194: [ELF] - Linkerscript: add support for suffixes in numbers.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 2 08:05:35 PDT 2016


ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:1235
@@ -1234,1 +1234,3 @@
 
+static bool readInteger(StringRef Tok, uint64_t &Result) {
+  enum {
----------------
Something's not right with this function. You seems to be mixing two different concepts, base and multiplier in this function, which confused me while reading this code. You cannot use K/M suffixes for hex numbers, so hex numbers should be handled first.

The code should be something like this.

  if (Tok.startswith_lower("0x"))
    return Tok.substr(2).getAsInteger(16, Result);
  if (Tok.endswith_lower("H"))
    return Tok.drop_back().getAsInteger(16, Result);

  int Suffix = 1;
  if (Tok.endswith_lower("K")) {
    Suffix = 1024;
    Tok = Tok.drop_back();
  } else if (Tok.endswith_lower("M")) {
    Suffix = 1024 * 1024;
    Tok = Tok.drop_back();
  }
  if (Tok.getAsInteger(10, Result))
    return false;
  Result *= Suffix;
  return true;


================
Comment at: ELF/LinkerScript.cpp:1236
@@ +1235,3 @@
+static bool readInteger(StringRef Tok, uint64_t &Result) {
+  enum {
+    Dec = 10,
----------------
Delete this enum and use these numbers directly. They will never change.


https://reviews.llvm.org/D24194





More information about the llvm-commits mailing list