factor: fix integer validation and GMP fallback

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 case can be detected independently of overflow.
Return an error when an empty string is passed.
Also allow leading spaces and '+' in input numbers.
* tests/misc/factor.pl: Ensure the GMP code is exercised
when compiled in. Also add a test to verify leading
spaces and '+' are allowed.

2 files changed, 45 insertions, 6 deletions

diff --git a/src/factor.c b/src/factor.c
index d5f248a6a..73c59e9b9 100644
--- a/src/factor.c
+++ b/src/factor.c
@@ -2222,14 +2222,30 @@ static strtol_error
 strto2uintmax (uintmax_t *hip, uintmax_t *lop, const char *s)
 {
   unsigned int lo_carry;
-  uintmax_t hi, lo;
+  uintmax_t hi = 0, lo = 0;
 
-  strtol_error err;
+  strtol_error err = LONGINT_INVALID;
 
-  hi = lo = 0;
+  /* Skip initial spaces and '+'.  */
   for (;;)
     {
-      unsigned int c = *s++;
+      char c = *s;
+      if (c == ' ')
+        s++;
+      else if (c == '+')
+        {
+          s++;
+          break;
+        }
+      else
+        break;
+    }
+
+  /* Initial scan for invalid digits.  */
+  const char *p = s;
+  for (;;)
+    {
+      unsigned int c = *p++;
       if (c == 0)
         break;
 
@@ -2238,9 +2254,17 @@ strto2uintmax (uintmax_t *hip, uintmax_t *lop, const char *s)
           err = LONGINT_INVALID;
           break;
         }
-      c -= '0';
 
       err = LONGINT_OK;           /* we've seen at least one valid digit */
+    }
+
+  for (;err == LONGINT_OK;)
+    {
+      unsigned int c = *s++;
+      if (c == 0)
+        break;
+
+      c -= '0';
 
       if (UNLIKELY (hi > ~(uintmax_t)0 / 10))
         {
@@ -2343,6 +2367,10 @@ print_factors (const char *input)
         }
       break;
 
+    case LONGINT_OVERFLOW:
+      /* Try GMP.  */
+      break;
+
     default:
       error (0, 0, _("%s is not a valid positive integer"), quote (input));
       return false;
diff --git a/tests/misc/factor.pl b/tests/misc/factor.pl
index 38a503714..8f6edaac4 100755
--- a/tests/misc/factor.pl
+++ b/tests/misc/factor.pl
@@ -28,6 +28,7 @@ my @Tests =
     (
      ['1', '9',          {OUT => '3 3'}],
      ['1a', '7',         {OUT => '7'}],
+     ['1b', '  +7',      {OUT => '7'}],
      ['2', '4294967291', {OUT => '4294967291'}],
      ['3', '4294967292', {OUT => '2 2 3 3 7 11 31 151 331'}],
      ['4', '4294967293', {OUT => '9241 464773'}],
@@ -74,6 +75,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;
@@ -81,7 +92,7 @@ my $t;
 Test:
 foreach $t (@Tests)
   {
-    my $arg1 = $t->[1];
+    (my $arg1 = $t->[1]) =~ s| *\+?||;
 
     # Don't fiddle with expected OUT string if there's a nonzero exit status.
     foreach my $e (@$t)