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

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:1235
@@ -1234,1 +1234,3 @@
 
+static bool readInteger(StringRef Tok, uint64_t &Result) {
+  enum {
----------------
ruiu wrote:
> 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;
> 
Done.


https://reviews.llvm.org/D24194