Re: [PATCH 25/41] builtin/receive-pack: avoid hard-coded constants for push certs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux