Re: [PATCH 01/17] cache: update object ID functions for the_hash_algo
On Sun, Jul 08, 2018 at 09:31:42PM -0700, Jacob Keller wrote:
> On Sun, Jul 8, 2018 at 9:05 PM Eric Sunshine wrote:
> >
> > On Sun, Jul 8, 2018 at 10:38 PM Jacob Keller wrote:
> > > On Sun, Jul 8, 2018 at 4:39 PM brian m. carlson
> > > wrote:
> > > > static inline int oidcmp(const struct object_id *oid1, const struct
> > > > object_id *oid2)
> > > > {
> > > > - return hashcmp(oid1->hash, oid2->hash);
> > > > + return memcmp(oid1->hash, oid2->hash, the_hash_algo->rawsz);
> > > > }
> > >
> > > Just curious, what's the reasoning for not using the hashcmp anymore?
> >
> > hashcmp() is specific to SHA-1 (for instance, it hardocdes
> > GIT_SHA1_RAWSZ). oidcmp() is meant as the hash-agnostic replacement
> > for hashcmp(), so it doesn't make sense to continue implementing
> > oidcmp() in terms of hashcmp() (the latter of which will eventually be
> > retired, presumably).
>
> Fair. I just saw that hashcmp was also updated to use the_hash_algo,
> but if we're going to drop it eventually, then there's zero reason to
> keep implementing oidcmp in terms of it, so... makes sense to me!
Actually, this reminded me that I have a patch that I had forgotten
about in my next series that updates hashcmp. I'll squash that in in my
reroll, and undo this change.
As a bonus, it also has a nicer commit message which I will include
explaining why this is necessary.
--
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
signature.asc
Description: PGP signature
Re: [PATCH 01/17] cache: update object ID functions for the_hash_algo
On Sun, Jul 8, 2018 at 9:05 PM Eric Sunshine wrote:
>
> On Sun, Jul 8, 2018 at 10:38 PM Jacob Keller wrote:
> > On Sun, Jul 8, 2018 at 4:39 PM brian m. carlson
> > wrote:
> > > static inline int oidcmp(const struct object_id *oid1, const struct
> > > object_id *oid2)
> > > {
> > > - return hashcmp(oid1->hash, oid2->hash);
> > > + return memcmp(oid1->hash, oid2->hash, the_hash_algo->rawsz);
> > > }
> >
> > Just curious, what's the reasoning for not using the hashcmp anymore?
>
> hashcmp() is specific to SHA-1 (for instance, it hardocdes
> GIT_SHA1_RAWSZ). oidcmp() is meant as the hash-agnostic replacement
> for hashcmp(), so it doesn't make sense to continue implementing
> oidcmp() in terms of hashcmp() (the latter of which will eventually be
> retired, presumably).
Fair. I just saw that hashcmp was also updated to use the_hash_algo,
but if we're going to drop it eventually, then there's zero reason to
keep implementing oidcmp in terms of it, so... makes sense to me!
Thanks,
Jake
Re: [PATCH 01/17] cache: update object ID functions for the_hash_algo
On Sun, Jul 8, 2018 at 10:38 PM Jacob Keller wrote:
> On Sun, Jul 8, 2018 at 4:39 PM brian m. carlson
> wrote:
> > static inline int oidcmp(const struct object_id *oid1, const struct
> > object_id *oid2)
> > {
> > - return hashcmp(oid1->hash, oid2->hash);
> > + return memcmp(oid1->hash, oid2->hash, the_hash_algo->rawsz);
> > }
>
> Just curious, what's the reasoning for not using the hashcmp anymore?
hashcmp() is specific to SHA-1 (for instance, it hardocdes
GIT_SHA1_RAWSZ). oidcmp() is meant as the hash-agnostic replacement
for hashcmp(), so it doesn't make sense to continue implementing
oidcmp() in terms of hashcmp() (the latter of which will eventually be
retired, presumably).
Re: [PATCH 01/17] cache: update object ID functions for the_hash_algo
On Sun, Jul 8, 2018 at 4:39 PM brian m. carlson
wrote:
> static inline int oidcmp(const struct object_id *oid1, const struct
> object_id *oid2)
> {
> - return hashcmp(oid1->hash, oid2->hash);
> + return memcmp(oid1->hash, oid2->hash, the_hash_algo->rawsz);
> }
>
Just curious, what's the reasoning for not using the hashcmp anymore?
Thanks,
Jake
[PATCH 01/17] cache: update object ID functions for the_hash_algo
Update the hashcpy and hashclr functions to use the_hash_algo, since
they are used in a variety of places to copy and manipulate buffers that
need to move data into or out of struct object_id. Update oidcmp so
that it is implemented on its own and similarly uses the_hash_algo.
Signed-off-by: brian m. carlson
---
cache.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/cache.h b/cache.h
index d49092d94d..c4a64278a1 100644
--- a/cache.h
+++ b/cache.h
@@ -977,7 +977,7 @@ static inline int hashcmp(const unsigned char *sha1, const
unsigned char *sha2)
static inline int oidcmp(const struct object_id *oid1, const struct object_id
*oid2)
{
- return hashcmp(oid1->hash, oid2->hash);
+ return memcmp(oid1->hash, oid2->hash, the_hash_algo->rawsz);
}
static inline int is_null_sha1(const unsigned char *sha1)
@@ -992,7 +992,7 @@ static inline int is_null_oid(const struct object_id *oid)
static inline void hashcpy(unsigned char *sha_dst, const unsigned char
*sha_src)
{
- memcpy(sha_dst, sha_src, GIT_SHA1_RAWSZ);
+ memcpy(sha_dst, sha_src, the_hash_algo->rawsz);
}
static inline void oidcpy(struct object_id *dst, const struct object_id *src)
@@ -1009,7 +1009,7 @@ static inline struct object_id *oiddup(const struct
object_id *src)
static inline void hashclr(unsigned char *hash)
{
- memset(hash, 0, GIT_SHA1_RAWSZ);
+ memset(hash, 0, the_hash_algo->rawsz);
}
static inline void oidclr(struct object_id *oid)
