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 <meyer...@redhat.com>
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 <meyer...@redhat.com>
+
+ 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 <t...@gmplib.org>
* 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
