[PATCH 2 of 2 resend] revlog: switch to a C version of headrevs

[Antoine Pitrou]

On Sat, 12 May 2012 19:13:46 +0200
Bryan O'Sullivan <bos at serpentine.com> wrote:
>  
> +	if (self->headrevs) {
> +		Py_DECREF(self->headrevs);
> +		self->headrevs = NULL;
> +	}

For the record, the Py_CLEAR() macro does this in a safer way.

> +static PyObject *index_headrevs(indexObject *self)
> +{
> +	Py_ssize_t i, len, addlen;
> +	char *ishead = NULL;
> +	PyObject *heads;
> +
> +	if (self->headrevs)
> +		return list_copy(self->headrevs);

Is there a risk of this being called from multiple threads at once?
Looks like the caching could leak objects if self->headrevs is
calculated by different threads concurrently.
(that's said it's unlikely for the GIL to be released during that
sequence, so you're probably protected by it)

> +		PyObject *nullid = PyInt_FromLong(-1);
> +		if (nullid == NULL || PyList_Append(heads, nullid) == -1)
> +			goto bail;

You'll be leaking nullid if PyList_Append() fails, but that's not
important since in practice PyInt_FromLong(-1) is interned.

> +		if (head == NULL || PyList_Append(heads, head) == -1)
> +			goto bail;

Same comment here, and `head` won't be interned in the general case.

Regards

Antoine.