Re: [kvm-unit-tests PATCH v2 1/4] lib/string: Add strnlen, strrchr and strtoul

Andrew Jones
2021-03-23T12:15:31Z

On Mon, Mar 22, 2021 at 09:35:23AM +0100, Andrew Jones wrote:
> @@ -208,23 +209,46 @@ unsigned long int strtoul(const char *nptr, char **endptr, int base)
> c = *s - 'A' + 10;
> else
> break;
> - acc = acc * base + c;
> +
> + if (is_signed) {
> + long __acc = (long)acc;
> + overflow = __builtin_smull_overflow(__acc, base, &__acc);
> + assert(!overflow);
> + overflow = __builtin_saddl_overflow(__acc, c, &__acc);
> + assert(!overflow);
> + acc = (unsigned long)__acc;
> + } else {
> + overflow = __builtin_umull_overflow(acc, base, &acc);
> + assert(!overflow);
> + overflow = __builtin_uaddl_overflow(acc, c, &acc);
> + assert(!overflow);
> + }
> +

Unfortunately my use of these builtins isn't loved by older compilers,
like the one used by the build-centos7 pipeline in our gitlab CI. I
could wrap them in an #if GCC_VERSION >= 50100 and just have the old
'acc = acc * base + c' as the fallback, but that's not pretty and
would also mean that clang would use the fallback too. Maybe we can
try and make our compiler.h more fancy in order to provide a
COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW define like linux does for
both gcc and clang. Or, we could just forgot the overflow checking.

Anybody else have suggestions? Paolo? Thomas?

Thanks,
drew