[PATCH 6 of 6 cpychecker] parsers: avoid over-retaining Py_None if index_init fails
Yuya Nishihara
yuya at tcha.org
Thu Aug 20 10:26:44 CDT 2015
On Tue, 18 Aug 2015 17:54:16 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <augie at google.com>
> # Date 1439931329 14400
> # Tue Aug 18 16:55:29 2015 -0400
> # Node ID 87c8ccbc26ccaddf628f2d09323d4664cc8066eb
> # Parent b9c2bab361f6abe80f7a155ba7910cf0a6bd90d8
> parsers: avoid over-retaining Py_None if index_init fails
>
> Spotted with cpychecker. Note that cpychecker still thinks there's a
> problem with over-retaining Py_None here, but I'm pretty sure it's
> wrong by inspection.
>
> I also noticed that self->data could end up over-retained, so I fixed
> that as well. cpychecker is silent both before and after that change,
> and I'm pretty sure it's right.
>
> diff --git a/mercurial/parsers.c b/mercurial/parsers.c
> --- a/mercurial/parsers.c
> +++ b/mercurial/parsers.c
> @@ -2349,15 +2349,15 @@ static int index_init(indexObject *self,
> self->data = NULL;
> self->headrevs = NULL;
> self->filteredrevs = Py_None;
> - Py_INCREF(Py_None);
> + Py_INCREF(self->filteredrevs);
> self->nt = NULL;
> self->offsets = NULL;
>
> if (!PyArg_ParseTuple(args, "OO", &data_obj, &inlined_obj))
> - return -1;
> + goto bail;
> if (!PyString_Check(data_obj)) {
> PyErr_SetString(PyExc_TypeError, "data is not a string");
> - return -1;
> + goto bail;
> }
> size = PyString_GET_SIZE(data_obj);
>
> @@ -2387,6 +2387,8 @@ static int index_init(indexObject *self,
>
> return 0;
> bail:
> + Py_DECREF(self->filteredrevs);
> + Py_XDECREF(self->data);
Perhaps this doubles decrefs because the index object should exist no matter
if index_init() fail and index_dealloc() would be called anyway?
The other patches look good to me, but I'm newcomer to C API.
More information about the Mercurial-devel
mailing list