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

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added inline comments.


================
Comment at: lib/scudo/standalone/string_utils.cc:17
+
+INLINE bool isSpace(int C) {
+  return (C == ' ') || (C == '\n') || (C == '\t') || (C == '\f') ||
----------------
Can we use `isspace` from libc?


================
Comment at: lib/scudo/standalone/string_utils.cc:22
+
+INLINE bool isDigit(int C) { return (C >= '0') && (C <= '9'); }
+
----------------
Can we use `isdigit` from libc?


================
Comment at: lib/scudo/standalone/string_utils.cc:24
+
+INLINE int convertToLower(int C) {
+  return (C >= 'A' && C <= 'Z') ? (C + 'a' - 'A') : C;
----------------
Can we use `tolower` from libc?


================
Comment at: lib/scudo/standalone/string_utils.cc:28
+
+s64 convertStringToS64(const char *NPtr, const char **EndPtr, int Base) {
+  CHECK_EQ(Base, 10);
----------------
`strtoll` should be safe here.  I can't think of a good reason for it to malloc anything.


================
Comment at: lib/scudo/standalone/string_utils.h:32
+  }
+  void append(const char *Format, va_list Args);
+  void append(const char *Format, ...);
----------------
Why do we need this append with `va_list`?


================
Comment at: lib/scudo/standalone/tests/strings_test.cc:13
+#include <limits.h>
+
+TEST(ScudoStringsTest, Basic) {
----------------
Can we add a test that the `ScopedString` buffer won't be overflowed by `append`?


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D59262