[PATCH] D59262: [scudo][standalone] Add string utility functions

Kostya Kortchinsky via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 12 11:28:36 PDT 2019


cryptoad added inline comments.


================
Comment at: lib/scudo/standalone/string_utils.cc:56
+
+s64 convertStringToS64(const char *NPtr, const char **EndPtr, int Base) {
+  CHECK_EQ(Base, 10);
----------------
hctim wrote:
> Personally, input params before output params, `EndPtr` should be after `Base`.
This one was mostly to mirror the behavior & parameters of https://linux.die.net/man/3/strtol
I am open to changing it if it makes sense to everyone.


================
Comment at: lib/scudo/standalone/string_utils.cc:73
+    Res = (Res <= UINT64_MAX / 10) ? Res * 10 : UINT64_MAX;
+    const uptr Digit = ((*NPtr) - '0');
+    Res = (Res <= UINT64_MAX - Digit) ? Res + Digit : UINT64_MAX;
----------------
hctim wrote:
> Why `uptr`? `char` or `int` should be sufficient. Also `const` not needed here.
Changed to `int`. `char` errors with `-Werror=conversion`.
As for `const`, I'd rather keep for consistency.


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59262/new/

https://reviews.llvm.org/D59262





More information about the llvm-commits mailing list