Re: [PATCH] fast-import: use hashcmp() for SHA1 hash comparison
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Jul 19, 2014, 16:43 UTC
- Message-ID
- <20140719164324.GA5616@vauxhall.crustytoothpaste.net>
- In-Reply-To
- <53CA6072.8000009@web.de>
On Sat, Jul 19, 2014 at 02:11:30PM +0200, René Scharfe wrote:
> I'd say if a platform doesn't bother optimizing memcmp() then they > deserve the resulting performance. And it's probably not too bad a > penalty because such comparisons probably won't make up a significant > part of most applications.
I tend to agree with this. On many modern versions of GCC, the compiler can generate an appropriately optimized inline version when it sees a memcmp call, so it's more of a compiler issue then, since no actual call to the function will be emitted.
> static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)
> {
> + const uint32_t *p1 = (const uint32_t *)sha1;
> + const uint32_t *p2 = (const uint32_t *)sha2;You can't make this cast. The guaranteed alignment for sha1 and sha2 is 1, and for p1 and p2, it's 4. If sha1 and sha2 are not suitably aligned, this will get a SIGBUS on sparc and possibly a wrong value on ARM[0].
[0] http://www.aleph1.co.uk/chapter-10-arm-structured-alignment-faq
-- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187