[libc-commits] [PATCH] D107999: [libc] Add strtol, strtoul, and strtoull

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Mon Aug 16 15:21:43 PDT 2021

sivachandra added inline comments.

Comment at: libc/src/__support/str_conv_utils.h:105
There is change in behavior from the earlier code. Even after detecting that the result is out of range we advance the pointer now, which was not the case previously because of the `break` statements. But I think this change is doing it right - it is reading out the full number even if it is out of range. So, may be add a test for it?

Also, can we reduce the loop body once overflow is detected? As in, before line 92, can we have something like:

if (result == ABS_MAX)

Comment at: libc/src/__support/str_conv_utils.h:120
+  return (result_sign == '+') ? static_cast<T>(result)
+                              : -static_cast<T>(result);
When the sign is negative and result is `ABS_MAX` and the return type is a signed integer type, then I think we have to handle that specially. Consider this: T is `long`, `result_sign` is `-` and `result` is `ABS_MAX` which is effectively `LONG_MAX + 1`. Since `LONG_MAX + 1` cannot be represent as a `long` type, the `static_cast` here becomes an implementation defined operation.

I think previously also it was implementation defined.

Comment at: libc/test/src/stdlib/strtoll_test.cpp:162
   char small_string[4] = {'\0', '\0', '\0', '\0'};
-  for (int base = 2; base <= 36; ++base) {
+  for (unsigned int base = 2; base <= 36; ++base) {
     for (long long first_digit = 0; first_digit <= 36; ++first_digit) {
Why did you change this to `unsigned` here and other similar places?

  rG LLVM Github Monorepo



More information about the libc-commits mailing list