[PATCH 3 of 7] parsers: avoid leaking obj in index_ancestors

[Martin von Zweigbergk]

On Fri Jan 23 2015 at 1:10:58 PM Augie Fackler <raf at durin42.com> wrote:

> # HG changeset patch
> # User Augie Fackler <augie at google.com>
> # Date 1422045207 18000
> #      Fri Jan 23 15:33:27 2015 -0500
> # Branch stable
> # Node ID 72365a74930dfeae8b7e7bce3e2beef6d6d95c5f
> # Parent  5de7357d24e4e5444198e82ab710dbffb9453d0a
> parsers: avoid leaking obj in index_ancestors
>
> PySequence_GetItem returns a new reference. Found with cpychecker.
>
> diff --git a/mercurial/parsers.c b/mercurial/parsers.c
> --- a/mercurial/parsers.c
> +++ b/mercurial/parsers.c
> @@ -1691,9 +1691,11 @@ static PyObject *index_ancestors(indexOb
>                 if (!PyInt_Check(obj)) {
>                         PyErr_SetString(PyExc_TypeError,
>                                         "arguments must all be ints");
> +                       Py_DECREF(obj);
>

Nit: Could move this call to beginning of block since 'obj' is not used.


>                         goto bail;
>                 }
>                 val = PyInt_AsLong(obj);
> +               Py_DECREF(obj);
>                 if (val == -1) {
>                         ret = PyList_New(0);
>                         goto done;
>

Unrelated to this patch, but do you know why -1 results in a successful
return of an empty list? -1 from PyInt_AsLong means that an error _might_
have happened and one should check PyErr_Occurred() to see whether it
actually did. We don't check that here. Perhaps its just very unlikely
something did go wrong, but even so, why would we return an empty list when
val == -1 rather than falling through and raising a PyExc_IndexError? Maybe
-1 was supposed to mean something special? I can't make any sense of that
from what the function is supposed to return. Also, all tests pass after
removing the block. Maybe I'll just send a separate patch removing it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150123/ae84fa66/attachment.html>