Re: [PATCH] sanity check in add_packed_git()

From: Junio C Hamano <junkio@cox.net>
Date: Dec 22, 2005, 01:42 UTC
Message-ID: <7v7j9xvrf3.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <1135208829.15567.18.camel@dv>

Pavel Roskin <proski@gnu.org> writes:

> add_packed_git() tries to get the pack SHA1 by parsing its name.  It may
> access uninitialized memory for packs with short names.

Thanks.

This fixes when there is ".git/objects/packs/junk-X.pack", so in that sense it is a real fix and I'll take it.

However, I feel a bit uneasy about this whole thing. The core does not care how you name your packs, as long as .pack and .idx files have the same prefix, but we started relying on the prefix being "pack-" followed by 40 hexadecimal digits since we started packed repository support in http-fetch, and we also allowed sha1_pack_name() function that shares the assumption to sneak into the real core part of the system around the same time (end of July 2005). git-repack and git-verify-pack stay ignorant about this prefix convention and I think that is a good property. However, we might be better off if we declare that the pack files *must* be named following that convention (currently nobody enforces it, but as long as the user uses "git repack" to create packs, the packs automatically follow that convention), and make check_packed_git_idx() reject a packfile whose name does not begin with "pack-" or the 40 hexdigits that follow does not match the hash of the object names the index records. If we go that route, then this patch becomes unneeded; more extensive check needs to be done in check_packed_git_idx().

Thoughts?