hash: Make code agnostic of endianness

[real-or-random]

Recent compilers compile the two new functions to very efficient code
on various platforms. In particular, already GCC >= 5 and clang >= 5
understand do this for the read function, which is the one critical
for performance (called 16 times per SHA256 transform).

Fixes #1080.

[real-or-random]

See https://gcc.godbolt.org/z/5ovzd49z1. We can benchmark this but I don't think it's necessary to be honest. SHA256 performance anyway is not our primary goal (at least with the current code)

[sipa]

utACK 37d3692

[robot-dreams]

ACK 37d3692

Glad you caught the int promotion issue when shifting. Sanity checked by searching for strings BE32, ENDIAN, chunk, ->buf to confirm nothing else needed to be changed. Benchmark anecdata on my machine looks reasonable:

# 1093
hash_sha256                   ,     0.197     ,     0.198     ,     0.202
hash_hmac_sha256              ,     0.786     ,     0.788     ,     0.798
hash_rfc6979_hmac_sha256      ,     4.29      ,     4.30      ,     4.31

# master
hash_sha256                   ,     0.212     ,     0.214     ,     0.216
hash_hmac_sha256              ,     0.841     ,     0.846     ,     0.854
hash_rfc6979_hmac_sha256      ,     4.30      ,     4.50      ,     4.64

[JeremyRubin]

can you add more parens around the cast

[real-or-random]

I can do it in a follow up PR if you insist but the code is correct given C's operator precedence.

[JeremyRubin]

like this:

(((uint32_t)p[0]) << 24) |
(((uint32_t)p[1]) << 16) |
(((uint32_t)p[2]) << 8) |
((uint32_t)p[3])

is much easier to read IMO

[real-or-random]

Yeah, I agree, it may be easier to review for some reviewers because they don't need to to spend time looking up the rules. But given that it has been merged (i.e., review has been done), I don't think it's worth spending a PR on this. Feel free to open a PR if you disagree.

(Sorry, I hadn't seen your comment earlier. I was spending some time testing this locally before pushing the merge. Your comment arrived after I had created the merge commit locally, but I believe before I pushed it. Then I didn't check github again before pushing the merge.)

[elichai]

Post merge review ACK 37d3692
Good to see less ifdefs as a side effect :)