Re: [IGNORETHIS/PATCH] Choosing the sha1 prefix of your commits

On Wed, Oct 19, 2011 at 11:57:17PM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

Agreed. Having hidden cruft makes birthday collision attacks easier (or
it will, if sha1 ever gets broken to that point).  Unfortunately, there
is a _ton_ of code which assumes that commit messages are
NUL-terminated, as they always have been since e871b64 (2005-05-25).

I think that commit is irrelevant, as long as read_sha1_file() returns the
contents as <ptr,len> pair, which has been the case forever. It's just the
matter of propagating the length back up the callchain.

It's not that the commit is bad or the source of problems. My point is
that the assumption that commit messages are NUL-terminated has been
there for a really long time, so there are lots of spots in the code
that sloppily run string functions on them. Every one of those needs to
be found and fixed (e.g., I remember seeing this in
for-each-ref.c:find_subpos recently).

It's not impossible, of course, or even really that hard. It's just a
giant pain, and I wonder if the effort is worth it.

A naïve implementation to add "len" member to struct commit would increase
the size of the in-core commit object by sizeof(unsigned long), which we
may want to avoid. Traversals that care nothing but the topology of the
history would have to waste that memory and these things tend to add up
(8-byte ulong * 250k commits = 2MB).

Perhaps change the type of "buf" member in struct commit to a pointer to a
<ptr,len> pair, or something? Or perhaps a few megabytes wasted between
friends we do not care much about?

I think you'd have to convert the struct (even if not every piece of
code is converted to use it) and profile.

-Peff