bug#12350: Composites identified as primes in factor.c (when HAVE_GMP)

[Jim Meyering]

Jim Meyering wrote:
> Torbjorn Granlund wrote:
>> We won't be sending any more code replacement blobs to this address; it
>> is most surely the wrong place.
>
> Hi Torbjorn,
>
> I guess you're saying that because there's been too little feedback?
> IMHO, this is great work.
> I've been reviewing the latest and had prepared several patches.
> Just hadn't made time to send them.
>
>> Please get our suggested factor.c replacement from
>> <http://gmplib.org:8000/factoring/>.
>>
>> I plan to spend no more time on this project now.  Should the
>> contribution be accepted, I will make the necessary amendments to the
>
> You may consider it accepted.  That was clear in my mind from
> the beginning.  Sorry if I didn't make that clear to you.
> Now, it's just a matter of integrating it.
>
>> GNU copyright paperwork.  I am certainly willing to answer questions
>> about the code, of course.
>
> Here are some suggested changes -- I made these against
> a temporary local git repository using your -005 tarball.
> That was before I learned (just now) that you have a mercurial
> repository.

Here's a change without which ourseq misuse clobbers the heap:

    $ valgrind ./ourseq 99999999999999999999999 1
    ==7387== Memcheck, a memory error detector
    ==7387== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
    ==7387== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
    ==7387== Command: ./ourseq 99999999999999999999999 1
    ==7387==
    ==7387== Invalid write of size 8
    ==7387==    at 0x4A0A0CB: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:837)
    ==7387==    by 0x4006CC: main (ourseq.c:74)
    ==7387==  Address 0x4c35040 is 0 bytes inside a block of size 3 alloc'd
    ==7387==    at 0x4A0884D: malloc (vg_replace_malloc.c:263)
    ==7387==    by 0x4006AD: main (ourseq.c:71)
    ==7387==
    first string greater than second string
    ==7387==
    ==7387== HEAP SUMMARY:
    ==7387==     in use at exit: 5 bytes in 2 blocks
    ==7387==   total heap usage: 2 allocs, 0 frees, 5 bytes allocated
    ==7387==
    ==7387== LEAK SUMMARY:
    ==7387==    definitely lost: 0 bytes in 0 blocks
    ==7387==    indirectly lost: 0 bytes in 0 blocks
    ==7387==      possibly lost: 0 bytes in 0 blocks
    ==7387==    still reachable: 5 bytes in 2 blocks
    ==7387==         suppressed: 0 bytes in 0 blocks
    ==7387== Rerun with --leak-check=full to see details of leaked memory
    ==7387==
    ==7387== For counts of detected and suppressed errors, rerun with: -v
    ==7387== ERROR SUMMARY: 3 errors from 1 contexts (suppressed: 2 from 2)


>From 2fe3143867a3a2f0b3f4d0ff71c8bfca9c676127 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 13 Sep 2012 13:27:48 +0200
Subject: [PATCH] bug fix: ourseq would clobber heap for some out-of-order
 args

* ourseq.c (main): Don't clobber heap for argv[1] longer than argv[2].
Also declare functions static and some parameters const.
---
 ChangeLog |  6 ++++++
 ourseq.c  | 13 +++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d71a8c3..b009136 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2012-09-13  Jim Meyering  <address@hidden>
+
+       bug fix: ourseq would clobber heap for some out-of-order args
+       * ourseq.c (main): Don't clobber heap for argv[1] longer than argv[2].
+       Also declare functions static and some parameters const.
+
 2012-09-10  Torbjorn Granlund  <address@hidden>

        * factor.c (mp_prime_p): Clear out `factors' only after Lucas run.
diff --git a/ourseq.c b/ourseq.c
index cb71f13..a9899f9 100644
--- a/ourseq.c
+++ b/ourseq.c
@@ -1,5 +1,5 @@
 /* A simple seq program that operates directly on the numeric strings.
-   This works around strange limits/bugs in standards seq implementations.  */
+   This works around strange limits/bugs in standard seq implementations.  */

 #include <stdlib.h>
 #include <string.h>
@@ -13,8 +13,8 @@ struct string

 typedef struct string string;

-int
-cmp (string *s1, string *s2)
+static int
+cmp (string const *s1, string const *s2)
 {
   size_t len1, len2;

@@ -29,7 +29,7 @@ cmp (string *s1, string *s2)
   return strcmp (s1->str, s2->str);
 }

-void
+static void
 incr (string *st)
 {
   size_t len;
@@ -67,6 +67,11 @@ main (int argc, char **argv)

   len1 = strlen (argv[1]);
   len2 = strlen (argv[2]);
+  if (len2 < len1)
+    {
+      fprintf (stderr, "first string greater than second string\n");
+      exit (1);
+    }

   b.str = malloc (len2 + 2);   /* not a typo for len1 */
   e.str = malloc (len2 + 1);
--
1.7.12.363.g53284de