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

[Jim Meyering]

Andreas Gruenbacher wrote:
> On Tuesday 24 May 2011 12:02:02 Jim Meyering wrote:
>> I ran coverity against patch and it reported this legitimate leak.
>> It was even triggered by the 'merge" test.
>
> looks correct, but can we use the existing exit path in this function, like 
> this?
>
> diff --git a/src/bestmatch.h b/src/bestmatch.h
> index 958b1ca..e49f7b4 100644
> --- a/src/bestmatch.h
> +++ b/src/bestmatch.h
> @@ -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 done;
> +       }
>        }
>      else
>        fmid_plus_2_min = 0;  /* disable this check */

The main difference is that returning directly does not set *PY when it
is non-NULL.  I preferred to leave *PY unset, since the fact that we're
returning max+1 already implies that *PY is not useful.  Either way works.

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.