[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