From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Thu, 20 Oct 2011 15:36:00 GMT
Subject: Re: [PATCH 3/6] revert: fix buffer overflow in insn sheet parser
Message-ID: <CALkWK0nQD7sw506D7shE8TYe5xGLNJkzS5_6L-oOR7gT54PbAA@mail.gmail.com>
In-Reply-To: <20111020090912.GA21471@elie.hsd1.il.comcast.net>

Hi Jonathan,

Jonathan Nieder writes:
> [...]
> Looks good, except I would explain it differently, to avoid referring
> to hypothetical implementation details ("What buffer overflow?"):
>
>        test: git cherry-pick --continue should cope with long object names
>
>        A naive implementation that uses a commit-id-shaped buffer
>        to store the word after "pick" in .git/sequencer/todo lines
>        would crash often.  Our implementation is not so naive, but
>        add a test anyway to futureproof it.
> [...]

I picked this one.

> Though the implementation is actually better than that --- it can even
> cope with a valid object name (e.g., a long name of a branch, or
> something like "HEAD^{/refs.c: ensure struct whose member}") that is
> that long, without truncating it.  So if you have time for it, I think
> it would be worth a test where the "git cherry-pick --continue"
> succeeds, too.

Good idea.  Will re-roll shortly.

Thanks.

-- Ram