Re: [PATCH] factor: fix integer validation and GMP fallback

[Pádraig Brady]

On 10/08/2012 06:10 PM, Jim Meyering wrote:
> Jim Meyering wrote:
> > Pádraig Brady wrote:
> > > In the recent factor rewrite, the GMP code
> > > wasn't actually used; just an error was printed
> > > on integer overflow.  While fixing that it was noticed
> > > that correct input validation wasn't done in all cases
> > > when falling back to the GMP code.
> > >
> > > * src/factor.c (print_factors) Fallback to GMP on overflow.
> > > (strto2uintmax): Scan the string for invalid characters,
> > > so that can be detected independently of overflow.
> > > Return an error when an empty string is passed.
> >
> > Oh, wow.  I'm sure glad you caught that.  Thanks!
> > I could have sworn there was a test that used GMP when possible.
> > I'll add one ASAP.
>
> Thanks again.
> You're welcome to merge this into your patch.
> Don't bother with attribution.
>
> diff --git a/tests/misc/factor.pl b/tests/misc/factor.pl
> index 38a5037..bfd48d1 100755
> --- a/tests/misc/factor.pl
> +++ b/tests/misc/factor.pl
> @@ -74,6 +74,16 @@ my @Tests =
>        ['bug-2012-e', '17754345703', {OUT => '94219 188437'}],
>       );
>
> +# If we have GMP support, append tests to exercise it.
> +system "grep -w HAVE_GMP $ENV{CONFIG_HEADER} > /dev/null" == 0
> +  and push (@Tests,
> +            ['bug-gmp-2_sup_128', '340282366920938463463374607431768211456',
> +             {OUT => '2 'x127 . '2'}],
> +            ['bug-gmp-2_sup_256',
> +             '115792089237316195423570985008687907853'
> +             . '269984665640564039457584007913129639936',
> +             {OUT => '2 'x255 . '2'}]);
> +
>   # Prepend the command line argument and append a newline to end
>   # of each expected 'OUT' string.
>   my $t;

Thanks for that.
I also notice that the old factor code
allowed leading spaces and an optional '+'
for input numbers.

I'll tweak to allow those too and add a test.

cheers,
Pádraig.