On Tue, Jul 30, 2019 at 03:49:38PM -0400, Todd Zullinger wrote:
> > Subtest 6 had an ordering issue. We do not know whether
> > the problem is the code or the test result not keeping up
> > with the code changes.
> >
> > --- expect 2019-07-30 16:56:36 +0000
> > +++ actual 2019-07-30 16:56:36 +0000
> > @@ -1,6 +1,6 @@
> > NULL
> > NULL
> > NULL
> > +7c7cd714e262561f73f3079dfca4e8724682ac21 3
> > 139b20d8e6c5b496de61f033f642d0e3dbff528d 2
> > d79ce1670bdcb76e6d1da2ae095e890ccb326ae9 1
> > -7c7cd714e262561f73f3079dfca4e8724682ac21 3
>
> I hit the same failure while building for Fedora on the
> s390x architecture. I have not dug into it much yet, but
> perhaps this is an endianess issue?
Ah, of course. Our oid hashing is done by just picking off the first
bytes of the sha1, and it doesn't care about endianness (because these
are just internal-to-memory hashes).
We _could_ reconcile that like this:
diff --git a/hashmap.h b/hashmap.h
index 8424911566..493229ac54 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -116,19 +116,11 @@ unsigned int memihash_cont(unsigned int hash_seed, const void *buf, size_t len);
* Converts a cryptographic hash (e.g. SHA-1) into an int-sized hash code
* for use in hash tables. Cryptographic hashes are supposed to have
* uniform distribution, so in contrast to `memhash()`, this just copies
- * the first `sizeof(int)` bytes without shuffling any bits. Note that
- * the results will be different on big-endian and little-endian
- * platforms, so they should not be stored or transferred over the net.
+ * the first `sizeof(int)` bytes without shuffling any bits.
*/
static inline unsigned int oidhash(const struct object_id *oid)
{
- /*
- * Equivalent to 'return *(unsigned int *)oid->hash;', but safe on
- * platforms that don't support unaligned reads.
- */
- unsigned int hash;
- memcpy(&hash, oid->hash, sizeof(hash));
- return hash;
+ return get_be32(oid->hash);
}
/*
diff --git a/t/t0016-oidmap.sh b/t/t0016-oidmap.sh
index bbe719e950..6656db9d69 100755
--- a/t/t0016-oidmap.sh
+++ b/t/t0016-oidmap.sh
@@ -93,9 +93,9 @@ put three 3
iterate" "NULL
NULL
NULL
+$(git rev-parse three) 3
$(git rev-parse two) 2
-$(git rev-parse one) 1
-$(git rev-parse three) 3"
+$(git rev-parse one) 1"
'
which not only fixes this test but any other hash-based oddities. I
wonder if it's appreciably less efficient. I'll bet I could nerd-snipe
René into doing a bunch of measurements and explorations of the
disassembled code. ;)
-Peff
