From: Eric Sunshine <ericsunshine@gmail.com>
Date: Wed, 13 Jul 2011 14:26:28 GMT
Subject: Re: [RFC/PATCHv2 5/6] check commit generation cache validity against grafts
Message-ID: <4E1DAB14.5040001@gmail.com>
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