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

Sean Farley sean.michael.farley at gmail.com
Fri Jan 9 16:19:56 CST 2015


Augie Fackler writes:

> 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.

Yeah, I have to agree with Sid that I had trouble reading "lzm".
We have no limit on function names (I'm looking at you, fortran), so
'lazymanifest_init_real' is fine with me. Emacs has autocompletion so
typing it shouldn't be too much a problem.

>>> +	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.

It does require declarations at the top. You're probably finding newer
docs fro VC{10 or higher} that implement some esoteric C99. I've cleaned
up the C89-isms and pushed them to smf.io/hg


More information about the Mercurial-devel mailing list