On Tue, Apr 24, 2018 at 11:58:17AM +0200, Martin Ă…gren wrote: > On 24 April 2018 at 01:39, brian m. carlson > > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c > > index c4272fbc96..5f35596c14 100644 > > --- a/builtin/receive-pack.c > > +++ b/builtin/receive-pack.c > > @@ -454,21 +454,21 @@ static void hmac_sha1(unsigned char *out, > > /* RFC 2104 2. (6) & (7) */ > > git_SHA1_Init(&ctx); > > git_SHA1_Update(&ctx, k_opad, sizeof(k_opad)); > > - git_SHA1_Update(&ctx, out, 20); > > + git_SHA1_Update(&ctx, out, GIT_SHA1_RAWSZ); > > git_SHA1_Final(out, &ctx); > > } > > Since we do HMAC with SHA-1, we use the functions `git_SHA1_foo()`. Ok. > But then why not just use "20"? Isn't GIT_SHA1_RAWSZ coupled to the > whole hash transition thing? This use of "20" is not, IMHO, the "length > in bytes [...] of an object name" (quoting cache.h). Originally, GIT_SHA1_RAWSZ was a good stand-in for the hard-coded uses of 20 (and GIT_SHA1_HEXSZ for 40) for object IDs. Recently, we've started moving toward using the_hash_algo for the object ID-specific hash values, so I've started using those constants only to identify SHA-1 specific items. In this case, using the constant makes it more obvious that what we're passing is indeed an SHA-1 hash. It also makes it easier to find all the remaining instances of "20" in the codebase and analyze them accordingly. I agree that this isn't an object name strictly, but it's essentially equivalent. If you feel strongly, I can leave this the way it is. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204