Re: [bug-patch] [PATCH] plug a leak

[Jim Meyering]

Andreas Gruenbacher wrote:

> On Wednesday 25 May 2011 15:41:47 Jim Meyering wrote:
>> Andreas Gruenbacher wrote:
>> > On Wednesday 25 May 2011 15:12:35 Jim Meyering wrote:
>> >> I can see the advantage of using the goto (single point of return and *PY 
>> >> is
>> >> always set).  However, using the goto suggests you'd have to update the
>> >> description to say what *PY = -1 means, but will any caller even care?
>> >> Probably not, but I haven't checked that.
>> >
>> > If a value > max is returned, *py is ignored.  Not sure there is much 
>> > value in
>> > explicitly documenting this ...
>>
>> I now realize why I chose not to modify *PY:
>> Returning MAX+1 is all that is necessary.  Doing any more (like
>> setting *PY) is best avoided in general (keep API's minimalist),
>> because some caller might be tempted to test *PY rather than the
>> return value.
>
> I'm fine either way, but please don't duplicate the free() and use a goto 
> instead.
>
>> I propose to document that:
>>
>> diff --git a/src/bestmatch.h b/src/bestmatch.h
>> index a162bb5..02f961c 100644
>> --- a/src/bestmatch.h
>> +++ b/src/bestmatch.h
>> @@ -36,7 +36,7 @@
>>   *
>>   * Returns the number of changes (insertions and deletions) required to get
>>   * from a[] to b[].  Returns MAX + 1 if a[] cannot be turned into b[] with
>> - * MAX or fewer changes.
>> + * MAX or fewer changes, in which case *PY is not modified.
>>   *
>>   * MIN specifies the minimum number of elements in which a[] and b[] must
>>   * match. This allows to prevent trivial matches in which a sequence is
>
> Okay.

Here's the result:

>From 5a634c3834b189ccafe0ba62caae3534c86994cc Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 24 May 2011 11:47:25 +0200
Subject: [PATCH] plug a leak in bestmatch

* src/bestmatch.h (bestmatch): Don't leak V when returning early.
---
 src/bestmatch.h |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/bestmatch.h b/src/bestmatch.h
index 958b1ca..1c91e16 100644
--- a/src/bestmatch.h
+++ b/src/bestmatch.h
@@ -36,7 +36,7 @@
  *
  * Returns the number of changes (insertions and deletions) required to get
  * from a[] to b[].  Returns MAX + 1 if a[] cannot be turned into b[] with
- * MAX or fewer changes.
+ * MAX or fewer changes, in which case *PY is not modified.
  *
  * MIN specifies the minimum number of elements in which a[] and b[] must
  * match. This allows to prevent trivial matches in which a sequence is
@@ -89,7 +89,10 @@ bestmatch(OFFSET xoff, OFFSET xlim, OFFSET yoff, OFFSET ylim,
        fmid_plus_2_min = fmid + 2 * min;
        min += yoff;
        if (min > ylim)
-         return max + 1;
+         {
+           c = max + 1;
+           goto free_and_return;
+         }
       }
     else
       fmid_plus_2_min = 0;  /* disable this check */
@@ -153,6 +156,8 @@ bestmatch(OFFSET xoff, OFFSET xlim, OFFSET yoff, OFFSET 
ylim,
   done:
     if (py)
       *py = ymax;
+
+  free_and_return:
     free (V);
     return c;
 }
--
1.7.5.2.609.g6dbbf