Re: [RFC/PATCHv2 5/6] check commit generation cache validity against grafts

From: Eric Sunshine <ericsunshine@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Jakub Narebski" <jnareb@gmail.com>, "Ted Ts'o" <tytso@mit.edu>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Clemens Buchacher" <drizzd@aon.at>,
	"Shawn O. Pearce" <spearce@spearce.org>
Subject: Re: [RFC/PATCHv2 5/6] check commit generation cache validity against grafts
Date: Wed, 13 Jul 2011 10:26:28 -0400	[thread overview]
Message-ID: <4E1DAB14.5040001@gmail.com> (raw)
In-Reply-To: <20110713070616.GE18566@sigill.intra.peff.net>

On 7/13/2011 3:06 AM, Jeff King wrote:
> +void metadata_graph_validity(unsigned char out[20])
> +{
> +	git_SHA_CTX ctx;
> +
> +	git_SHA1_Init(&ctx);
> +
> +	git_SHA1_Update(&ctx, "grafts", 6);
> +	commit_graft_validity(&ctx);
> +
> +	git_SHA1_Update(&ctx, "replace", 7);
> +	replace_object_validity(&ctx);

The implementation of metadata_graph_validity() makes it clear that 
commit_graft_validity() and replace_object_validity() are computing 
checksums in aid of validity-checking of the generations cache. However, 
the naive reader seeing the names commit_graft_validity() and 
replace_object_validity() in the API is likely to assume that these 
functions are somehow checking validity of the grafts and replace-refs 
themselves, which is not the case. Perhaps better names would be 
commit_graft_checksum() and replace_object_checksum()?

The name metadata_graph_validity() also suffers from this shortcoming. 
The actual validity check is performed by check_cache_header(), whereas 
metadata_graph_validity() is merely computing a checksum.

-- ES