[PATCH 3 of 3] imported patch revlog_c_parseindex.patch

[Matt Mackall]

On Tue, 2008-10-14 at 00:03 +0200, Bernhard Leiner wrote:
> +	/* TODO DRY!
> +	 * if possible, do not hardcode node.nullid and node.nullrev here */
> +	char nullrev[20] = {'\0','\0','\0','\0','\0','\0','\0','\0','\0','\0',
> +			    '\0','\0','\0','\0','\0','\0','\0','\0','\0','\0'};

const char nullrev[20]; /* compiler preinitializes static values to zero
*/

> +	int nullid = -1;
> +
> +	while (data < end) {
> +		/* create new revlog record */
> +		r = PyTuple_New(8);

It'd probably be tidier to:

- unpack everything to C types first
- create all the relevant Python objects
- one test for success
- pack everything into the tuple and list

> +		if (!r)
> +			goto fail_record;
> +
> +		/* get offset and flags */
> +		offset_flags = ntohll(*((uint64_t *)data));
> +		if (n == 0) {
> +			offset_flags &= 0xFFFF; /* mask out version number */
> +		}
> +		py_obj = PyLong_FromUnsignedLongLong(offset_flags);

Does this give a regular int for smaller values? The size difference is
important.

> +	/* create Python objects to be returned */
> +	rval = PyTuple_New(3);
> +	if (!rval)
> +		goto fail_rval;
> +	if (inlined) {
> +		index = PyList_New(0);
> +	} else {
> +		/* In this case we know the size of the index list in advance:
> +		 * size divided by size of one one revlog record (64 bytes) */  
> +		index = PyList_New(size / 64);
> +	}

You should probably deal with the magic index[nullrev] entry here too.

-- 
Mathematics is the supreme nostalgia of our time.