[libc-commits] [PATCH] D108330: [libc] add atoi, atol, and atoll
Michael Jones via Phabricator via libc-commits
libc-commits at lists.llvm.org
Wed Aug 18 16:59:10 PDT 2021
michaelrj added inline comments.
Comment at: libc/test/src/stdlib/atol_test.cpp:42
+ ASSERT_EQ(__llvm_libc::atol(all_together), 110l);
> No `LONG_MAX` tests?
> Also, test for overflow values in all tests? We don't need to check the `errno` value but may be check that the return value is `*_MAX`/`*_MIN`?
I've been avoiding `LONG_MAX` in the tests, since the size of `LONG_MAX` changes based on the operating system, specifically Windows uses 32 bit `long` while linux uses 64 bit `long`. This is difficult when I'm hardcoding the strings.
There aren't any overflow value tests since overflowing is undefined behavior for `atoi`. I can add them, but it means we're testing things that aren't in the specification.
Comment at: libc/test/src/stdlib/atol_test.cpp:48
+ const char *octal = "010";
+ ASSERT_EQ(__llvm_libc::atol(hexadecimal), 10l);
> Is this really an invalid number?
I've changed the name of the tests, `010` isn't an invalid number, but it is interpreted as an octal number if the base is set to 0.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libc-commits