[libc-commits] [PATCH] D107999: [libc] change internal strtoll to be templated

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Fri Aug 13 09:11:27 PDT 2021

sivachandra added a comment.

Can you add implementation of `strtol` and `strtoul` and tests for them in this same patch? They will help us catch problems in the corner cases if any (one of which I have commented about inline).

Comment at: libc/src/__support/str_conv_utils.h:78-81
+  unsigned long long const NEGATIVE_MAX =
+      (__llvm_libc::cpp::NumericLimits<T>::min() != 0)
+          ? __llvm_libc::cpp::NumericLimits<T>::min()
+          : __llvm_libc::cpp::NumericLimits<T>::max();
I am not sure this is doing what you want. Lets take the example of `T` being the type `int`. I think you want `NEGATIVE_MAX` to be `INT_MAX` + 1. But, the above code is effectively:

unsigned long long const NEGATIVE_MASK = INT_MIN;

Which makes `NEGATIVE_MASK` a number much larger than `INT_MAX + 1`. This should fix it:

  unsigned long long const NEGATIVE_MAX =
      (__llvm_libc::cpp::NumericLimits<T>::min() != 0)
          ? static_cast<unsigned long long>(__llvm_libc::cpp::NumericLimits<T>::max()) + 1
          : __llvm_libc::cpp::NumericLimits<T>::max();

Comment at: libc/src/__support/str_conv_utils.h:111
-    return -result;
+    return -(T)(result);
We should prefer `T(...)` or `static_cast<T>(...)` over c-style casts. Also, use the ternary operator instead of an `if`-`else` statement?

  rG LLVM Github Monorepo



More information about the libc-commits mailing list