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

Andrew Jones
2021-03-23T13:32:43Z

On Tue, Mar 23, 2021 at 02:01:47PM +0100, Thomas Huth wrote:
> On 23/03/2021 13.14, Andrew Jones wrote:
> > 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?
>
> What does a "normal" libc implementation do (e.g. glibc)? If it is also not
> doing overflow checking, I think we also don't need it in the
> kvm-unit-tests.
>

You'll get LONG_MAX for strtol and ULONG_MAX for strtoul and errno will be
set to ERANGE.

Thanks,
drew