On 25 April 2018 at 04:00, brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: > 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. I see. So one could say that in the ideal end-game, GIT_SHA1_RAWSZ would be gone when the oid-hash-transition is over. Except since we also use SHA-1 for other stuff than object IDs, the real-world ideal end-game is that we only have a few users lingering in places that have nothing to do with oid, but only with SHA-1 (and maybe in the gluing for calculating SHA-1 oids..). I do not feel strongly about this. I was just surprised to see it. Thank you for explaining this. Martin
