[PATCH stable] parsers.c: fix a couple of memory leaks
Sean Farley
sean.michael.farley at gmail.com
Thu Jun 12 17:14:07 CDT 2014
Siddharth Agarwal writes:
> On 06/11/2014 09:44 PM, Sean Farley wrote:
>> Kevin Bullock writes:
>>
>>> On Jun 11, 2014, at 5:43 PM, danek.duvall at oracle.com wrote:
>>>
>>>> # HG changeset patch
>>>> # User Danek Duvall <danek.duvall at oracle.com>
>>>> # Date 1402525864 25200
>>>> # Wed Jun 11 15:31:04 2014 -0700
>>>> # Branch stable
>>>> # Node ID b091b262aa6d25e7e4663672daf2b6d8f127ab48
>>>> # Parent b35f8c487e396487e89f98e92da57ac5eb9833af
>>>> parsers.c: fix a couple of memory leaks
>>>>
>>>> diff --git a/mercurial/parsers.c b/mercurial/parsers.c
>>>> --- a/mercurial/parsers.c
>>>> +++ b/mercurial/parsers.c
>>>> @@ -1403,8 +1403,12 @@ static PyObject *find_deepest(indexObjec
>>>> final |= i;
>>>> j -= 1;
>>>> }
>>>> - if (final == 0)
>>>> + if (final == 0) {
>>>> + free(depth);
>>>> + free(seen);
>>>> + free(interesting);
>>>> return PyList_New(0);
>>>> + }
>>> Good catch, but this is now the 3rd place we've got those same `free()`s in this function. Would something like this work instead:
>>>
>>> diff --git a/mercurial/parsers.c b/mercurial/parsers.c
>>> --- a/mercurial/parsers.c
>>> +++ b/mercurial/parsers.c
>>> @@ -1403,8 +1403,10 @@ static PyObject *find_deepest(indexObjec
>>> final |= i;
>>> j -= 1;
>>> }
>>> - if (final == 0)
>>> - return PyList_New(0);
>>> + if (final == 0) {
>>> + keys = PyList_New(0);
>>> + goto cleanup;
>> http://xkcd.com/292/
>
> As Greg said, goto is absolutely the right choice for error handling in
> C. I generally don't consider forward gotos to be a problem. It is
> *backward* gotos that get really hairy.
Sorry. I actually agree with both you and Greg. I just couldn't help
make the reference to xkcd :-)
More information about the Mercurial-devel
mailing list