Skip to content

Commit

Permalink
Fix issues with a big endian platform
Browse files Browse the repository at this point in the history
  • Loading branch information
@cr-marcstevens
cr-marcstevens committed May 15, 2017
1 parent 5ee29e5 commit 33a694a
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lib/sha1.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
#if (!defined SHA1DC_FORCE_LITTLEENDIAN) && \
((defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \
(defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) || \
defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) || defined(__AARCH64EB__) || \
defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) || defined(__AARCH64EB__) || \
defined(_MIPSEB) || defined(__MIPSEB) || defined(__MIPSEB__) || defined(SHA1DC_FORCE_BIGENDIAN))

#define SHA1DC_BIGENDIAN
Expand Down Expand Up @@ -1744,7 +1744,8 @@ void SHA1DCUpdate(SHA1_CTX* ctx, const char* buf, size_t len)
while (len >= 64)
{
ctx->total += 64;
sha1_process(ctx, (uint32_t*)(buf));
memcpy(ctx->buffer, buf, 64);
sha1_process(ctx, (uint32_t*)(ctx->buffer));

This comment has been minimized.

Sign in to view

Copy link
@dscho

dscho May 16, 2017

This impacts performance. Maybe this could be done only for CPUs that require quad-aligned access?

This comment has been minimized.

Sign in to view

Copy link
@shumow

shumow May 16, 2017

Collaborator

Agreed, removing these memcpy's was a part of the performance work that I did. @cr-marcstevens which platform were you specifically trying to fix the problem with?

This comment has been minimized.

Sign in to view

Copy link
@dscho

dscho May 16, 2017

This change was made in reaction to a bug report where Git with DC_SHA1 would crash on SPARCs. Probably the same issue on ARM, as that architecture also requires data alignment. I guess the conservative thing would be to use the safe option except when we know it is unnecessary. Something like

#if defined(__amd64__) || defined(__amd64) || defined(__x86_64__) || defined(__x86_64)
    || defined(i386) || defined(__i386) || defined(__i386__) || defined(__i486__)
    || defined(__i586__) || defined(__i686__) || defined(_M_IX86) || defined(__X86__)
    || defined(_X86_) || defined(__THW_INTEL__) || defined(__I86__) || defined(__INTEL__)
    || defined(__386)
		/* These architectures support unaligned access */
		sha1_process(ctx, (uint32_t*)(buf));
#else
		memcpy(ctx->buffer, buf, 64);
		sha1_process(ctx, (uint32_t*)(ctx->buffer));
#endif

(I took this list of constants from https://sourceforge.net/p/predef/wiki/Architectures/).

This comment has been minimized.

Sign in to view

Copy link
@shumow

shumow May 16, 2017

Collaborator

Ok, here's a fix: 40f07a0

If Travis-CI passes, I'll create a pull request.
buf += 64;
len -= 64;
}
Expand Down

0 comments on commit 33a694a

Please sign in to comment.