[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