Re: [PATCH 2/5] revert: make commit subjects in insn sheet optional

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Dec 9, 2011, 19:44 UTC
Message-ID: <CALkWK0ki1r5AqYb8qyGHUNupTAhCa2TKwVgkrCpLr+zo_pCy9g@mail.gmail.com>
In-Reply-To: <20111209193527.GD20913@elie.hsd1.il.comcast.net>

Hi,

Jonathan Nieder wrote:

Show 21 quoted lines

> Ramkumar Ramachandra wrote:
>> Jonathan Nieder wrote:
>>> Ramkumar Ramachandra wrote:
>
>>>> [...]
>>>> While at it, also fix a bug: currently, we use a commit-id-shaped
>>>> buffer to store the word after "pick" in '.git/sequencer/todo'.  This
>>>> is both wasteful and wrong because it places an artificial limit on
>>>> the line length.  Eliminate the need for the buffer altogether, and
>>>> add a test demonstrating this.
>>>
>>> Reading the above does not make it at all obvious that I should want
>>> to apply this patch because otherwise my prankster friend can cause my
>>> copy of git to crash or run arbitrary code by putting a long commit
>>
>> Working backwards:
>> get_sha1() is what will finally misbehave: how?
>
> Not sure what you're talking about.  I was saying that any commit that
> goes from "git can segfault in such-and-such circumstance" to "git no
> longer segfaults in that circumstance" should loudly proclaim so!

That's what I'm confused about: how can I make git misbehave (let alone segfault) after applying this commit? In the previous message, I was trying to work backwards to figure that out -- I clearly didn't get anywhere. In other words: what happens if an overly long (a length that doesn't fit into a size_t) line is present? How does strchrnul() behave? How do we work around this without imposing some kind of artificial hard limit?

Thanks.

-- Ram