[PATCH 1 of 4 lazy-manifest] manifest.c: new extension code to lazily parse manifests

Augie Fackler raf at durin42.com
Fri Jan 9 11:41:11 CST 2015


On Jan 8, 2015, at 11:52 PM, Siddharth Agarwal <sid at less-broken.com> wrote:

> Quick look:
> 
> On 01/08/2015 12:34 PM, Augie Fackler wrote:
>> # HG changeset patch
>> # User Augie Fackler <augie at google.com>
>> # Date 1420748959 18000
>> #      Thu Jan 08 15:29:19 2015 -0500
>> # Node ID 788bb2dacad048f161dfed583d3a95f708e91d1e
>> # Parent  7ad155e13f0f51df8e986a0ec4e58ac9a0ccedbb
>> manifest.c: new extension code to lazily parse manifests
>> 

>> +
>> +static int lzm_init_real(lazymanifest * self, PyObject * pydata) {
> 
> My brain read "lzm" as a compression algorithm in the Lempel-Ziv family.

I'm open to other suggestions for how to flag these method names.


>> +	Py_ssize_t consumed = pl + 41;
> 
> In C89 declarations need to be at the top of the block.

I've revised the comments. It *looks* like in most places we're on a reasonable compiler, and even VC9 (AFAICT) doesn't require declarations at the top like this. I cleaned it up in a couple of places before realizing that gcc won't let me use strnlen if I set -std=c89, but I know that's available in VC9. I've revised the comment further up about bool, and now use stdbool everywhere that's not VC9.

> 
>> +	PyObject *pyhash = PyTuple_GetItem(value, 0);
>> +	char *hash = "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0";
> 
> Why is this set to this value in particular? I tried following all the
> branches but quickly became cross-eyed.

This was only required when the hash was set to None, which no longer happens, so this will be gone in the next round. Nice catch.


>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150109/dc6191be/attachment.pgp>


More information about the Mercurial-devel mailing list