All of lore.kernel.org
 help / color / mirror / Atom feed
* git fsck failure on OS X with files >= 4 GiB
@ 2015-10-28 23:10 Rafael Espíndola
  2015-10-29  6:46 ` Filipe Cabecinhas
       [not found] ` <CAEDE8505fXAwVXx=EZwxPHvXpMByzpnXJ9LBgfx3U6VUaFbPHw@mail.gmail.com>
  0 siblings, 2 replies; 34+ messages in thread
From: Rafael Espíndola @ 2015-10-28 23:10 UTC (permalink / raw)
  To: git; +Cc: Filipe Cabecinhas

I first noticed this with "2.4.9 (Apple Git-60)", but it reproduces
with git built from 37023ba381b6d251d7140a997b39b566dbc63c42.

Create two files with just 0s:

-rw-r--r--  1 espindola  staff  4294967296 28 Oct 11:09 exactly-4gib
-rw-r--r--  1 espindola  staff  4294967295 28 Oct 11:09 one-less-than-4gib


and run

git init
git add one-less-than-4gib
git commit -m bar
git fsck
git add exactly-4gib
git commit -m bar
git fsck

The first fsck will run with no problems, but the second one fails:

error: packed cfdaf54c9ccfd8f5e4cee562f7d5f92df13d3106 from
.git/objects/pack/pack-ff08480fd7f767b6bd0aeb559f0f5dea2245b0b3.pack
is corrupt

Using the very same revision on freebsd doesn't cause any errors.

Cheers,
Rafael

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: git fsck failure on OS X with files >= 4 GiB
  2015-10-28 23:10 git fsck failure on OS X with files >= 4 GiB Rafael Espíndola
@ 2015-10-29  6:46 ` Filipe Cabecinhas
       [not found] ` <CAEDE8505fXAwVXx=EZwxPHvXpMByzpnXJ9LBgfx3U6VUaFbPHw@mail.gmail.com>
  1 sibling, 0 replies; 34+ messages in thread
From: Filipe Cabecinhas @ 2015-10-29  6:46 UTC (permalink / raw)
  To: Rafael Espíndola; +Cc: git

I did some debugging, and it seems CC_SHA1_Update (used by
write_sha1_file_prepare if APPLE_COMMON_CRYPTO is defined in the
Makefile) takes a uint32_t as a "length" parameter, which explains why
it stops working at 4GiB (UINT_MAX+1).

In the OS X 10.11 SDK header CommonCrypto/CommonDigest.h, we have:

typedef uint32_t CC_LONG;       /* 32 bit unsigned integer */
//...
extern int CC_SHA1_Update(CC_SHA1_CTX *c, const void *data, CC_LONG len)

A possible fix would be to either call SHA1_Update with a maximum of
UINT_MAX, looping if necessary. Or have a compatibility SHA1_Update
for OS X which can handle data longer than UINT_MAX.

I'm not sure what the git maintainers would prefer.

Regards,

  Filipe


On Wed, Oct 28, 2015 at 4:10 PM, Rafael Espíndola
<rafael.espindola@gmail.com> wrote:
>
> I first noticed this with "2.4.9 (Apple Git-60)", but it reproduces
> with git built from 37023ba381b6d251d7140a997b39b566dbc63c42.
>
> Create two files with just 0s:
>
> -rw-r--r--  1 espindola  staff  4294967296 28 Oct 11:09 exactly-4gib
> -rw-r--r--  1 espindola  staff  4294967295 28 Oct 11:09 one-less-than-4gib
>
>
> and run
>
> git init
> git add one-less-than-4gib
> git commit -m bar
> git fsck
> git add exactly-4gib
> git commit -m bar
> git fsck
>
> The first fsck will run with no problems, but the second one fails:
>
> error: packed cfdaf54c9ccfd8f5e4cee562f7d5f92df13d3106 from
> .git/objects/pack/pack-ff08480fd7f767b6bd0aeb559f0f5dea2245b0b3.pack
> is corrupt
>
> Using the very same revision on freebsd doesn't cause any errors.
>
> Cheers,
> Rafael

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: git fsck failure on OS X with files >= 4 GiB
       [not found] ` <CAEDE8505fXAwVXx=EZwxPHvXpMByzpnXJ9LBgfx3U6VUaFbPHw@mail.gmail.com>
@ 2015-10-29 10:46   ` Rafael Espíndola
  2015-10-29 15:15     ` Filipe Cabecinhas
  0 siblings, 1 reply; 34+ messages in thread
From: Rafael Espíndola @ 2015-10-29 10:46 UTC (permalink / raw)
  To: Filipe Cabecinhas; +Cc: git

Awesome, building with

NO_OPENSSL = 1
NO_GETTEXT = 1

produces a working git :-)

Cheers,
Rafael


On 28 October 2015 at 23:37, Filipe Cabecinhas <filcab@gmail.com> wrote:
> I did some debugging, and it seems CC_SHA1_Update (used by
> write_sha1_file_prepare if APPLE_COMMON_CRYPTO is defined in the Makefile)
> takes a uint32_t as a "length" parameter, which explains why it stops
> working at 4GiB (UINT_MAX+1).
>
> In the OS X 10.11 SDK header CommonCrypto/CommonDigest.h, we have:
>
> typedef uint32_t CC_LONG;       /* 32 bit unsigned integer */
> //...
> extern int CC_SHA1_Update(CC_SHA1_CTX *c, const void *data, CC_LONG len)
>
> A possible fix would be to either call SHA1_Update with a maximum of
> UINT_MAX, looping if necessary. Or have a compatibility SHA1_Update for OS X
> which can handle data longer than UINT_MAX.
>
> I'm not sure what the git maintainers would prefer.
>
> Regards,
>
>   Filipe
>
> On Wed, Oct 28, 2015 at 4:10 PM, Rafael Espíndola
> <rafael.espindola@gmail.com> wrote:
>>
>> I first noticed this with "2.4.9 (Apple Git-60)", but it reproduces
>> with git built from 37023ba381b6d251d7140a997b39b566dbc63c42.
>>
>> Create two files with just 0s:
>>
>> -rw-r--r--  1 espindola  staff  4294967296 28 Oct 11:09 exactly-4gib
>> -rw-r--r--  1 espindola  staff  4294967295 28 Oct 11:09 one-less-than-4gib
>>
>>
>> and run
>>
>> git init
>> git add one-less-than-4gib
>> git commit -m bar
>> git fsck
>> git add exactly-4gib
>> git commit -m bar
>> git fsck
>>
>> The first fsck will run with no problems, but the second one fails:
>>
>> error: packed cfdaf54c9ccfd8f5e4cee562f7d5f92df13d3106 from
>> .git/objects/pack/pack-ff08480fd7f767b6bd0aeb559f0f5dea2245b0b3.pack
>> is corrupt
>>
>> Using the very same revision on freebsd doesn't cause any errors.
>>
>> Cheers,
>> Rafael
>
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: git fsck failure on OS X with files >= 4 GiB
  2015-10-29 10:46   ` Rafael Espíndola
@ 2015-10-29 15:15     ` Filipe Cabecinhas
  2015-10-29 16:02       ` Atousa Duprat
  0 siblings, 1 reply; 34+ messages in thread
From: Filipe Cabecinhas @ 2015-10-29 15:15 UTC (permalink / raw)
  To: Rafael Espíndola; +Cc: git

Defining BLK_SHA1 = YesPlease (when calling make) should just change
the SHA functions, instead of completely removing OpenSSL or
CommonCrypto.

Regards,
  Filipe


On Thu, Oct 29, 2015 at 3:46 AM, Rafael Espíndola
<rafael.espindola@gmail.com> wrote:
> Awesome, building with
>
> NO_OPENSSL = 1
> NO_GETTEXT = 1
>
> produces a working git :-)
>
> Cheers,
> Rafael
>
>
> On 28 October 2015 at 23:37, Filipe Cabecinhas <filcab@gmail.com> wrote:
>> I did some debugging, and it seems CC_SHA1_Update (used by
>> write_sha1_file_prepare if APPLE_COMMON_CRYPTO is defined in the Makefile)
>> takes a uint32_t as a "length" parameter, which explains why it stops
>> working at 4GiB (UINT_MAX+1).
>>
>> In the OS X 10.11 SDK header CommonCrypto/CommonDigest.h, we have:
>>
>> typedef uint32_t CC_LONG;       /* 32 bit unsigned integer */
>> //...
>> extern int CC_SHA1_Update(CC_SHA1_CTX *c, const void *data, CC_LONG len)
>>
>> A possible fix would be to either call SHA1_Update with a maximum of
>> UINT_MAX, looping if necessary. Or have a compatibility SHA1_Update for OS X
>> which can handle data longer than UINT_MAX.
>>
>> I'm not sure what the git maintainers would prefer.
>>
>> Regards,
>>
>>   Filipe
>>
>> On Wed, Oct 28, 2015 at 4:10 PM, Rafael Espíndola
>> <rafael.espindola@gmail.com> wrote:
>>>
>>> I first noticed this with "2.4.9 (Apple Git-60)", but it reproduces
>>> with git built from 37023ba381b6d251d7140a997b39b566dbc63c42.
>>>
>>> Create two files with just 0s:
>>>
>>> -rw-r--r--  1 espindola  staff  4294967296 28 Oct 11:09 exactly-4gib
>>> -rw-r--r--  1 espindola  staff  4294967295 28 Oct 11:09 one-less-than-4gib
>>>
>>>
>>> and run
>>>
>>> git init
>>> git add one-less-than-4gib
>>> git commit -m bar
>>> git fsck
>>> git add exactly-4gib
>>> git commit -m bar
>>> git fsck
>>>
>>> The first fsck will run with no problems, but the second one fails:
>>>
>>> error: packed cfdaf54c9ccfd8f5e4cee562f7d5f92df13d3106 from
>>> .git/objects/pack/pack-ff08480fd7f767b6bd0aeb559f0f5dea2245b0b3.pack
>>> is corrupt
>>>
>>> Using the very same revision on freebsd doesn't cause any errors.
>>>
>>> Cheers,
>>> Rafael
>>
>>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: git fsck failure on OS X with files >= 4 GiB
  2015-10-29 15:15     ` Filipe Cabecinhas
@ 2015-10-29 16:02       ` Atousa Duprat
  2015-10-29 17:19         ` Junio C Hamano
  0 siblings, 1 reply; 34+ messages in thread
From: Atousa Duprat @ 2015-10-29 16:02 UTC (permalink / raw)
  To: git; +Cc: Rafael Espíndola, Filipe Cabecinhas

Here is a solution that avoids problems with OS-specific
implementations of SHA_Update() by limiting the size of each update
request to 1GiB and calling the function repeatedly in a loop.

Atousa

--

[PATCH] Limit the size of the data block passed to SHA1_Update()

This avoids issues where OS-specific implementations use
a 32-bit integer to specify block size.  Limit currently
set to 1GiB.
---
 cache.h | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index 79066e5..c305985 100644
--- a/cache.h
+++ b/cache.h
@@ -14,10 +14,28 @@
 #ifndef git_SHA_CTX
 #define git_SHA_CTX SHA_CTX
 #define git_SHA1_Init SHA1_Init
-#define git_SHA1_Update SHA1_Update
 #define git_SHA1_Final SHA1_Final
 #endif

+#define SHA1_MAX_BLOCK_SIZE (1024*1024*1024)
+
+static inline int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)
+{
+ size_t nr;
+ size_t total = 0;
+ char *cdata = (char*)data;
+ while(len > 0) {
+ nr = len;
+ if(nr > SHA1_MAX_BLOCK_SIZE)
+ nr = SHA1_MAX_BLOCK_SIZE;
+ SHA1_Update(c, cdata, nr);
+ total += nr;
+ cdata += nr;
+ len -= nr;
+ }
+ return total;
+}
+
 #include <zlib.h>
 typedef struct git_zstream {
  z_stream z;
-- 
2.4.9 (Apple Git-60)


On Thu, Oct 29, 2015 at 8:15 AM, Filipe Cabecinhas <filcab@gmail.com> wrote:
> Defining BLK_SHA1 = YesPlease (when calling make) should just change
> the SHA functions, instead of completely removing OpenSSL or
> CommonCrypto.
>
> Regards,
>   Filipe
>
>
> On Thu, Oct 29, 2015 at 3:46 AM, Rafael Espíndola
> <rafael.espindola@gmail.com> wrote:
>> Awesome, building with
>>
>> NO_OPENSSL = 1
>> NO_GETTEXT = 1
>>
>> produces a working git :-)
>>
>> Cheers,
>> Rafael
>>
>>
>> On 28 October 2015 at 23:37, Filipe Cabecinhas <filcab@gmail.com> wrote:
>>> I did some debugging, and it seems CC_SHA1_Update (used by
>>> write_sha1_file_prepare if APPLE_COMMON_CRYPTO is defined in the Makefile)
>>> takes a uint32_t as a "length" parameter, which explains why it stops
>>> working at 4GiB (UINT_MAX+1).
>>>
>>> In the OS X 10.11 SDK header CommonCrypto/CommonDigest.h, we have:
>>>
>>> typedef uint32_t CC_LONG;       /* 32 bit unsigned integer */
>>> //...
>>> extern int CC_SHA1_Update(CC_SHA1_CTX *c, const void *data, CC_LONG len)
>>>
>>> A possible fix would be to either call SHA1_Update with a maximum of
>>> UINT_MAX, looping if necessary. Or have a compatibility SHA1_Update for OS X
>>> which can handle data longer than UINT_MAX.
>>>
>>> I'm not sure what the git maintainers would prefer.
>>>
>>> Regards,
>>>
>>>   Filipe
>>>
>>> On Wed, Oct 28, 2015 at 4:10 PM, Rafael Espíndola
>>> <rafael.espindola@gmail.com> wrote:
>>>>
>>>> I first noticed this with "2.4.9 (Apple Git-60)", but it reproduces
>>>> with git built from 37023ba381b6d251d7140a997b39b566dbc63c42.
>>>>
>>>> Create two files with just 0s:
>>>>
>>>> -rw-r--r--  1 espindola  staff  4294967296 28 Oct 11:09 exactly-4gib
>>>> -rw-r--r--  1 espindola  staff  4294967295 28 Oct 11:09 one-less-than-4gib
>>>>
>>>>
>>>> and run
>>>>
>>>> git init
>>>> git add one-less-than-4gib
>>>> git commit -m bar
>>>> git fsck
>>>> git add exactly-4gib
>>>> git commit -m bar
>>>> git fsck
>>>>
>>>> The first fsck will run with no problems, but the second one fails:
>>>>
>>>> error: packed cfdaf54c9ccfd8f5e4cee562f7d5f92df13d3106 from
>>>> .git/objects/pack/pack-ff08480fd7f767b6bd0aeb559f0f5dea2245b0b3.pack
>>>> is corrupt
>>>>
>>>> Using the very same revision on freebsd doesn't cause any errors.
>>>>
>>>> Cheers,
>>>> Rafael
>>>
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Atousa Pahlevan, PhD
M.Math. University of Waterloo, Canada
Ph.D. Department of Computer Science, University of Victoria, Canada
Voice: 415-341-6206
Email: apahlevan@ieee.org
Website: www.apahlevan.org

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: git fsck failure on OS X with files >= 4 GiB
  2015-10-29 16:02       ` Atousa Duprat
@ 2015-10-29 17:19         ` Junio C Hamano
  2015-10-30  2:15           ` Atousa Duprat
  0 siblings, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2015-10-29 17:19 UTC (permalink / raw)
  To: Atousa Duprat; +Cc: git, Rafael Espíndola, Filipe Cabecinhas

Atousa Duprat <atousa.p@gmail.com> writes:

> [PATCH] Limit the size of the data block passed to SHA1_Update()
>
> This avoids issues where OS-specific implementations use
> a 32-bit integer to specify block size.  Limit currently
> set to 1GiB.
> ---
>  cache.h | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/cache.h b/cache.h
> index 79066e5..c305985 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -14,10 +14,28 @@
>  #ifndef git_SHA_CTX
>  #define git_SHA_CTX SHA_CTX
>  #define git_SHA1_Init SHA1_Init
> -#define git_SHA1_Update SHA1_Update
>  #define git_SHA1_Final SHA1_Final
>  #endif
>
> +#define SHA1_MAX_BLOCK_SIZE (1024*1024*1024)
> +
> +static inline int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)
> +{
> + size_t nr;
> + size_t total = 0;
> + char *cdata = (char*)data;
> + while(len > 0) {
> + nr = len;
> + if(nr > SHA1_MAX_BLOCK_SIZE)
> + nr = SHA1_MAX_BLOCK_SIZE;
> + SHA1_Update(c, cdata, nr);
> + total += nr;
> + cdata += nr;
> + len -= nr;
> + }
> + return total;
> +}
> +

I think the idea illustrated above is a good start, but there are
a few issues:

 * SHA1_Update() is used in fairly many places; it is unclear if it
   is a good idea to inline.

 * There is no need to punish implementations with working
   SHA1_Update by another level of wrapping.

 * What would you do when you find an implementation for which 1G is
   still too big?

Perhaps something like this in the header

#ifdef SHA1_MAX_BLOCK_SIZE
extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);
#define git_SHA1_Update SHA1_Update_Chunked
#endif

with compat/sha1_chunked.c that has

#ifdef SHA1_MAX_BLOCK_SIZE
int SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)
{
	... your looping implementation ...
}
#endif

in it, that is only triggered via a Makefile macro, e.g. 
might be a good workaround.

diff --git a/Makefile b/Makefile
index 8466333..83348b8 100644
--- a/Makefile
+++ b/Makefile
@@ -139,6 +139,10 @@ all::
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
+#
 # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
 #
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
@@ -1002,6 +1006,7 @@ ifeq ($(uname_S),Darwin)
 	ifndef NO_APPLE_COMMON_CRYPTO
 		APPLE_COMMON_CRYPTO = YesPlease
 		COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
+		SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L
 	endif
 	NO_REGEX = YesPlease
 	PTHREAD_LIBS =
@@ -1350,6 +1355,11 @@ endif
 endif
 endif
 
+ifdef SHA1_MAX_BLOCK_SIZE
+LIB_OBJS += compat/sha1_chunked.o
+BASIC_CFLAGS += SHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
+endif
+
 ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: git fsck failure on OS X with files >= 4 GiB
  2015-10-29 17:19         ` Junio C Hamano
@ 2015-10-30  2:15           ` Atousa Duprat
  2015-10-30 22:12             ` [PATCH] Limit the size of the data block passed to SHA1_Update() Atousa Pahlevan Duprat
  2015-10-30 22:18             ` Atousa Pahlevan Duprat
  0 siblings, 2 replies; 34+ messages in thread
From: Atousa Duprat @ 2015-10-30  2:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Rafael Espíndola, Filipe Cabecinhas

Thank you for the feedback.  I have revised the proposed patch as
suggested, allowing the use of SHA1_MAX_BLOCK_SIZE to enable the
chunked implementation.  When building for OSX with the CommonCrypto
library we error out if SHA1_MAX_BLOCK_SIZE is not defined, which will
avoid compiling a version of the tool that won't compute hashes
properly on large files.  It should be easy to enable this on other
platforms if needed.

Atousa

On Thu, Oct 29, 2015 at 10:19 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Atousa Duprat <atousa.p@gmail.com> writes:
>
>> [PATCH] Limit the size of the data block passed to SHA1_Update()
>>
>> This avoids issues where OS-specific implementations use
>> a 32-bit integer to specify block size.  Limit currently
>> set to 1GiB.
>> ---
>>  cache.h | 20 +++++++++++++++++++-
>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/cache.h b/cache.h
>> index 79066e5..c305985 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -14,10 +14,28 @@
>>  #ifndef git_SHA_CTX
>>  #define git_SHA_CTX SHA_CTX
>>  #define git_SHA1_Init SHA1_Init
>> -#define git_SHA1_Update SHA1_Update
>>  #define git_SHA1_Final SHA1_Final
>>  #endif
>>
>> +#define SHA1_MAX_BLOCK_SIZE (1024*1024*1024)
>> +
>> +static inline int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)
>> +{
>> + size_t nr;
>> + size_t total = 0;
>> + char *cdata = (char*)data;
>> + while(len > 0) {
>> + nr = len;
>> + if(nr > SHA1_MAX_BLOCK_SIZE)
>> + nr = SHA1_MAX_BLOCK_SIZE;
>> + SHA1_Update(c, cdata, nr);
>> + total += nr;
>> + cdata += nr;
>> + len -= nr;
>> + }
>> + return total;
>> +}
>> +
>
> I think the idea illustrated above is a good start, but there are
> a few issues:
>
>  * SHA1_Update() is used in fairly many places; it is unclear if it
>    is a good idea to inline.
>
>  * There is no need to punish implementations with working
>    SHA1_Update by another level of wrapping.
>
>  * What would you do when you find an implementation for which 1G is
>    still too big?
>
> Perhaps something like this in the header
>
> #ifdef SHA1_MAX_BLOCK_SIZE
> extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);
> #define git_SHA1_Update SHA1_Update_Chunked
> #endif
>
> with compat/sha1_chunked.c that has
>
> #ifdef SHA1_MAX_BLOCK_SIZE
> int SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)
> {
>         ... your looping implementation ...
> }
> #endif
>
> in it, that is only triggered via a Makefile macro, e.g.
> might be a good workaround.
>
> diff --git a/Makefile b/Makefile
> index 8466333..83348b8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -139,6 +139,10 @@ all::
>  # Define PPC_SHA1 environment variable when running make to make use of
>  # a bundled SHA1 routine optimized for PowerPC.
>  #
> +# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
> +# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
> +# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
> +#
>  # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
>  #
>  # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
> @@ -1002,6 +1006,7 @@ ifeq ($(uname_S),Darwin)
>         ifndef NO_APPLE_COMMON_CRYPTO
>                 APPLE_COMMON_CRYPTO = YesPlease
>                 COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
> +               SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L
>         endif
>         NO_REGEX = YesPlease
>         PTHREAD_LIBS =
> @@ -1350,6 +1355,11 @@ endif
>  endif
>  endif
>
> +ifdef SHA1_MAX_BLOCK_SIZE
> +LIB_OBJS += compat/sha1_chunked.o
> +BASIC_CFLAGS += SHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
> +endif
> +
>  ifdef NO_PERL_MAKEMAKER
>         export NO_PERL_MAKEMAKER
>  endif



-- 
Atousa Pahlevan, PhD
M.Math. University of Waterloo, Canada
Ph.D. Department of Computer Science, University of Victoria, Canada
Voice: 415-341-6206
Email: apahlevan@ieee.org
Website: www.apahlevan.org

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-10-30  2:15           ` Atousa Duprat
@ 2015-10-30 22:12             ` Atousa Pahlevan Duprat
  2015-10-30 22:22               ` Junio C Hamano
  2015-11-01  1:32               ` Eric Sunshine
  2015-10-30 22:18             ` Atousa Pahlevan Duprat
  1 sibling, 2 replies; 34+ messages in thread
From: Atousa Pahlevan Duprat @ 2015-10-30 22:12 UTC (permalink / raw)
  To: git; +Cc: Atousa Pahlevan Duprat

Some implementations of SHA_Updates have inherent limits
on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
to set the max chunk size supported, if required.  This is
enabled for OSX CommonCrypto library and set to 1GiB.
---
 Makefile                     |  9 +++++++++
 cache.h                      |  7 ++++++-
 compat/apple-common-crypto.h |  4 ++++
 compat/sha1_chunked.c        | 20 ++++++++++++++++++++
 4 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 compat/sha1_chunked.c

diff --git a/Makefile b/Makefile
index 04c2231..5955542 100644
--- a/Makefile
+++ b/Makefile
@@ -141,6 +141,10 @@ all::
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
+#
 # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
 #
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
@@ -1346,6 +1350,7 @@ else
 ifdef APPLE_COMMON_CRYPTO
 	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
 	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
+	SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
 else
 	SHA1_HEADER = <openssl/sha.h>
 	EXTLIBS += $(LIB_4_CRYPTO)
@@ -1353,6 +1358,10 @@ endif
 endif
 endif
 
+ifdef SHA1_MAX_BLOCK_SIZE
+	LIB_OBJS += compat/sha1_chunked.o
+	BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
+endif
 ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif
diff --git a/cache.h b/cache.h
index 79066e5..ec84b16 100644
--- a/cache.h
+++ b/cache.h
@@ -14,7 +14,12 @@
 #ifndef git_SHA_CTX
 #define git_SHA_CTX	SHA_CTX
 #define git_SHA1_Init	SHA1_Init
-#define git_SHA1_Update	SHA1_Update
+#ifdef SHA1_MAX_BLOCK_SIZE
+extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);
+#define git_SHA1_Update SHA1_Update_Chunked
+#else
+#define git_SHA1_Update SHA1_Update
+#endif
 #define git_SHA1_Final	SHA1_Final
 #endif
 
diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
index c8b9b0e..83668fd 100644
--- a/compat/apple-common-crypto.h
+++ b/compat/apple-common-crypto.h
@@ -16,6 +16,10 @@
 #undef TYPE_BOOL
 #endif
 
+#ifndef SHA1_MAX_BLOCK_SIZE
+#error "Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE"
+#endif
+
 #ifdef APPLE_LION_OR_NEWER
 #define git_CC_error_check(pattern, err) \
 	do { \
diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
new file mode 100644
index 0000000..4a8e4f7
--- /dev/null
+++ b/compat/sha1_chunked.c
@@ -0,0 +1,20 @@
+#include "cache.h"
+
+#ifdef SHA1_MAX_BLOCK_SIZE
+int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)
+{
+	size_t nr;
+	size_t total = 0;
+	char *cdata = (char*)data;
+	while(len > 0) {
+		nr = len;
+		if(nr > SHA1_MAX_BLOCK_SIZE)
+			nr = SHA1_MAX_BLOCK_SIZE;
+		SHA1_Update(c, cdata, nr);
+		total += nr;
+		cdata += nr;
+		len -= nr;
+	}
+	return total;
+}
+#endif
-- 
2.4.9 (Apple Git-60)

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-10-30  2:15           ` Atousa Duprat
  2015-10-30 22:12             ` [PATCH] Limit the size of the data block passed to SHA1_Update() Atousa Pahlevan Duprat
@ 2015-10-30 22:18             ` Atousa Pahlevan Duprat
  2015-10-30 22:26               ` Randall S. Becker
  1 sibling, 1 reply; 34+ messages in thread
From: Atousa Pahlevan Duprat @ 2015-10-30 22:18 UTC (permalink / raw)
  To: git; +Cc: Atousa Pahlevan Duprat

Some implementations of SHA_Updates have inherent limits
on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
to set the max chunk size supported, if required.  This is
enabled for OSX CommonCrypto library and set to 1GiB.
---
 Makefile                     |  9 +++++++++
 cache.h                      |  7 ++++++-
 compat/apple-common-crypto.h |  4 ++++
 compat/sha1_chunked.c        | 20 ++++++++++++++++++++
 4 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 compat/sha1_chunked.c

diff --git a/Makefile b/Makefile
index 04c2231..5955542 100644
--- a/Makefile
+++ b/Makefile
@@ -141,6 +141,10 @@ all::
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
+#
 # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
 #
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
@@ -1346,6 +1350,7 @@ else
 ifdef APPLE_COMMON_CRYPTO
 	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
 	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
+	SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
 else
 	SHA1_HEADER = <openssl/sha.h>
 	EXTLIBS += $(LIB_4_CRYPTO)
@@ -1353,6 +1358,10 @@ endif
 endif
 endif
 
+ifdef SHA1_MAX_BLOCK_SIZE
+	LIB_OBJS += compat/sha1_chunked.o
+	BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
+endif
 ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif
diff --git a/cache.h b/cache.h
index 79066e5..ec84b16 100644
--- a/cache.h
+++ b/cache.h
@@ -14,7 +14,12 @@
 #ifndef git_SHA_CTX
 #define git_SHA_CTX	SHA_CTX
 #define git_SHA1_Init	SHA1_Init
-#define git_SHA1_Update	SHA1_Update
+#ifdef SHA1_MAX_BLOCK_SIZE
+extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);
+#define git_SHA1_Update SHA1_Update_Chunked
+#else
+#define git_SHA1_Update SHA1_Update
+#endif
 #define git_SHA1_Final	SHA1_Final
 #endif
 
diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
index c8b9b0e..83668fd 100644
--- a/compat/apple-common-crypto.h
+++ b/compat/apple-common-crypto.h
@@ -16,6 +16,10 @@
 #undef TYPE_BOOL
 #endif
 
+#ifndef SHA1_MAX_BLOCK_SIZE
+#error "Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE"
+#endif
+
 #ifdef APPLE_LION_OR_NEWER
 #define git_CC_error_check(pattern, err) \
 	do { \
diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
new file mode 100644
index 0000000..4a8e4f7
--- /dev/null
+++ b/compat/sha1_chunked.c
@@ -0,0 +1,20 @@
+#include "cache.h"
+
+#ifdef SHA1_MAX_BLOCK_SIZE
+int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)
+{
+	size_t nr;
+	size_t total = 0;
+	char *cdata = (char*)data;
+	while(len > 0) {
+		nr = len;
+		if(nr > SHA1_MAX_BLOCK_SIZE)
+			nr = SHA1_MAX_BLOCK_SIZE;
+		SHA1_Update(c, cdata, nr);
+		total += nr;
+		cdata += nr;
+		len -= nr;
+	}
+	return total;
+}
+#endif
-- 
2.4.9 (Apple Git-60)

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-10-30 22:12             ` [PATCH] Limit the size of the data block passed to SHA1_Update() Atousa Pahlevan Duprat
@ 2015-10-30 22:22               ` Junio C Hamano
  2015-11-01  6:41                 ` Atousa Duprat
  2015-11-01  1:32               ` Eric Sunshine
  1 sibling, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2015-10-30 22:22 UTC (permalink / raw)
  To: Atousa Pahlevan Duprat; +Cc: git, Atousa Pahlevan Duprat

Atousa Pahlevan Duprat <atousa.p@gmail.com> writes:

> Some implementations of SHA_Updates have inherent limits
> on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
> to set the max chunk size supported, if required.  This is
> enabled for OSX CommonCrypto library and set to 1GiB.
> ---

Missing sign-off.

>  Makefile                     |  9 +++++++++
>  cache.h                      |  7 ++++++-
>  compat/apple-common-crypto.h |  4 ++++
>  compat/sha1_chunked.c        | 20 ++++++++++++++++++++
>  4 files changed, 39 insertions(+), 1 deletion(-)
>  create mode 100644 compat/sha1_chunked.c
>
> diff --git a/Makefile b/Makefile
> index 04c2231..5955542 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -141,6 +141,10 @@ all::
>  # Define PPC_SHA1 environment variable when running make to make use of
>  # a bundled SHA1 routine optimized for PowerPC.
>  #
> +# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
> +# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
> +# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
> +#
>  # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
>  #
>  # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
> @@ -1346,6 +1350,7 @@ else
>  ifdef APPLE_COMMON_CRYPTO
>  	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
>  	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
> +	SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
>  else
>  	SHA1_HEADER = <openssl/sha.h>
>  	EXTLIBS += $(LIB_4_CRYPTO)
> @@ -1353,6 +1358,10 @@ endif
>  endif
>  endif
>  
> +ifdef SHA1_MAX_BLOCK_SIZE
> +	LIB_OBJS += compat/sha1_chunked.o
> +	BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
> +endif
>  ifdef NO_PERL_MAKEMAKER
>  	export NO_PERL_MAKEMAKER
>  endif
> diff --git a/cache.h b/cache.h
> index 79066e5..ec84b16 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -14,7 +14,12 @@
>  #ifndef git_SHA_CTX
>  #define git_SHA_CTX	SHA_CTX
>  #define git_SHA1_Init	SHA1_Init
> -#define git_SHA1_Update	SHA1_Update
> +#ifdef SHA1_MAX_BLOCK_SIZE
> +extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);
> +#define git_SHA1_Update SHA1_Update_Chunked
> +#else
> +#define git_SHA1_Update SHA1_Update
> +#endif
>  #define git_SHA1_Final	SHA1_Final
>  #endif
>  
> diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
> index c8b9b0e..83668fd 100644
> --- a/compat/apple-common-crypto.h
> +++ b/compat/apple-common-crypto.h
> @@ -16,6 +16,10 @@
>  #undef TYPE_BOOL
>  #endif
>  
> +#ifndef SHA1_MAX_BLOCK_SIZE
> +#error "Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE"
> +#endif
> +

It crossed my mind if this might be better to just define it to some
reasonable value instead of erroring out, but because we do give a
default value in the Makefile, it would be a sign that the user is
doing something _quite_ unusual if the symbol is not defined here,
so I agree with your decision to error it out here.

>  #ifdef APPLE_LION_OR_NEWER
>  #define git_CC_error_check(pattern, err) \
>  	do { \
> diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
> new file mode 100644
> index 0000000..4a8e4f7
> --- /dev/null
> +++ b/compat/sha1_chunked.c
> @@ -0,0 +1,20 @@
> +#include "cache.h"
> +
> +#ifdef SHA1_MAX_BLOCK_SIZE
> +int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)
> +{
> +	size_t nr;
> +	size_t total = 0;
> +	char *cdata = (char*)data;

Please have a single blank line between the decls at the beginning
of a function and its first statement.  I am not sure about the cast
here, though.  Doesn't the function SHA1_Update() you are going to
call in the body of the loop take "const void *" as its second
parameter?  That's how openssl/sha1.h and block-sha1/sha1.h declare
this function.

> +	while(len > 0) {
> +		nr = len;
> +		if(nr > SHA1_MAX_BLOCK_SIZE)

Please have a SP around () for control statements, like while,
switch and if.

> +			nr = SHA1_MAX_BLOCK_SIZE;
> +		SHA1_Update(c, cdata, nr);
> +		total += nr;
> +		cdata += nr;
> +		len -= nr;
> +	}
> +	return total;
> +}
> +#endif

Thanks.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* RE: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-10-30 22:18             ` Atousa Pahlevan Duprat
@ 2015-10-30 22:26               ` Randall S. Becker
  2015-10-31 17:35                 ` Junio C Hamano
  2015-11-01  6:37                 ` Atousa Duprat
  0 siblings, 2 replies; 34+ messages in thread
From: Randall S. Becker @ 2015-10-30 22:26 UTC (permalink / raw)
  To: 'Atousa Pahlevan Duprat', git; +Cc: 'Atousa Pahlevan Duprat'

On October-30-15 6:18 PM, Atousa Pahlevan Duprat wrote:
>Some implementations of SHA_Updates have inherent limits on the max chunk
size. >SHA1_MAX_BLOCK_SIZE can be defined to set the max chunk size
supported, if >required.  This is enabled for OSX CommonCrypto library and
set to 1GiB.
>---
> <snip>

Didn't we have this same issue with NonStop port about a year ago and
decided to provision this through the configure script?

Cheers,
Randall

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-10-30 22:26               ` Randall S. Becker
@ 2015-10-31 17:35                 ` Junio C Hamano
  2015-11-01  6:37                 ` Atousa Duprat
  1 sibling, 0 replies; 34+ messages in thread
From: Junio C Hamano @ 2015-10-31 17:35 UTC (permalink / raw)
  To: Randall S. Becker
  Cc: 'Atousa Pahlevan Duprat', git, 'Atousa Pahlevan Duprat'

"Randall S. Becker" <rsbecker@nexbridge.com> writes:

> Didn't we have this same issue with NonStop port about a year ago and
> decided to provision this through the configure script?

I do not recall, but a support in configure.ac would be a nice
addition.

It does not have to be a requirement for the first cut solution,
though.  The customization based on Makefile variables like in
Atousa's change would serve as a foundation to add configure support
as a follow-up change, so let's not make the scope of this topic too
big before we have a solid foundation to build on.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-10-30 22:12             ` [PATCH] Limit the size of the data block passed to SHA1_Update() Atousa Pahlevan Duprat
  2015-10-30 22:22               ` Junio C Hamano
@ 2015-11-01  1:32               ` Eric Sunshine
  2015-11-01  6:32                 ` atousa.p
  1 sibling, 1 reply; 34+ messages in thread
From: Eric Sunshine @ 2015-11-01  1:32 UTC (permalink / raw)
  To: Atousa Pahlevan Duprat; +Cc: Git List, Atousa Pahlevan Duprat

On Fri, Oct 30, 2015 at 6:12 PM, Atousa Pahlevan Duprat
<atousa.p@gmail.com> wrote:
> Some implementations of SHA_Updates have inherent limits
> on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
> to set the max chunk size supported, if required.  This is
> enabled for OSX CommonCrypto library and set to 1GiB.
> ---
> diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
> index c8b9b0e..83668fd 100644
> --- a/compat/apple-common-crypto.h
> +++ b/compat/apple-common-crypto.h
> @@ -16,6 +16,10 @@
>  #undef TYPE_BOOL
>  #endif
>
> +#ifndef SHA1_MAX_BLOCK_SIZE
> +#error "Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE"
> +#endif

You can drop the quotes around the argument to #error.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-11-01  1:32               ` Eric Sunshine
@ 2015-11-01  6:32                 ` atousa.p
  2015-11-01  8:30                   ` Eric Sunshine
  2015-11-01 18:37                   ` Junio C Hamano
  0 siblings, 2 replies; 34+ messages in thread
From: atousa.p @ 2015-11-01  6:32 UTC (permalink / raw)
  To: git, sunshine, gitster, rsbecker; +Cc: Atousa Pahlevan Duprat

From: Atousa Pahlevan Duprat <apahlevan@ieee.org>

Some implementations of SHA_Updates have inherent limits
on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
to set the max chunk size supported, if required.  This is
enabled for OSX CommonCrypto library and set to 1GiB.

Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>
---
 Makefile                     |  9 +++++++++
 cache.h                      |  7 ++++++-
 compat/apple-common-crypto.h |  4 ++++
 compat/sha1_chunked.c        | 21 +++++++++++++++++++++
 4 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 compat/sha1_chunked.c

diff --git a/Makefile b/Makefile
index 04c2231..5955542 100644
--- a/Makefile
+++ b/Makefile
@@ -141,6 +141,10 @@ all::
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
+#
 # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
 #
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
@@ -1346,6 +1350,7 @@ else
 ifdef APPLE_COMMON_CRYPTO
 	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
 	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
+	SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
 else
 	SHA1_HEADER = <openssl/sha.h>
 	EXTLIBS += $(LIB_4_CRYPTO)
@@ -1353,6 +1358,10 @@ endif
 endif
 endif
 
+ifdef SHA1_MAX_BLOCK_SIZE
+	LIB_OBJS += compat/sha1_chunked.o
+	BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
+endif
 ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif
diff --git a/cache.h b/cache.h
index 79066e5..ec84b16 100644
--- a/cache.h
+++ b/cache.h
@@ -14,7 +14,12 @@
 #ifndef git_SHA_CTX
 #define git_SHA_CTX	SHA_CTX
 #define git_SHA1_Init	SHA1_Init
-#define git_SHA1_Update	SHA1_Update
+#ifdef SHA1_MAX_BLOCK_SIZE
+extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);
+#define git_SHA1_Update SHA1_Update_Chunked
+#else
+#define git_SHA1_Update SHA1_Update
+#endif
 #define git_SHA1_Final	SHA1_Final
 #endif
 
diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
index c8b9b0e..d3fb264 100644
--- a/compat/apple-common-crypto.h
+++ b/compat/apple-common-crypto.h
@@ -16,6 +16,10 @@
 #undef TYPE_BOOL
 #endif
 
+#ifndef SHA1_MAX_BLOCK_SIZE
+#error Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE
+#endif
+
 #ifdef APPLE_LION_OR_NEWER
 #define git_CC_error_check(pattern, err) \
 	do { \
diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
new file mode 100644
index 0000000..bf62b1b
--- /dev/null
+++ b/compat/sha1_chunked.c
@@ -0,0 +1,21 @@
+#include "cache.h"
+
+#ifdef SHA1_MAX_BLOCK_SIZE
+int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)
+{
+	size_t nr;
+	size_t total = 0;
+	char *cdata = (char*)data;
+
+	while (len > 0) {
+		nr = len;
+		if (nr > SHA1_MAX_BLOCK_SIZE)
+			nr = SHA1_MAX_BLOCK_SIZE;
+		SHA1_Update(c, cdata, nr);
+		total += nr;
+		cdata += nr;
+		len -= nr;
+	}
+	return total;
+}
+#endif
-- 
2.4.9 (Apple Git-60)

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-10-30 22:26               ` Randall S. Becker
  2015-10-31 17:35                 ` Junio C Hamano
@ 2015-11-01  6:37                 ` Atousa Duprat
  1 sibling, 0 replies; 34+ messages in thread
From: Atousa Duprat @ 2015-11-01  6:37 UTC (permalink / raw)
  To: Randall S. Becker; +Cc: git, Atousa Pahlevan Duprat

> Didn't we have this same issue with NonStop port about a year ago and
> decided to provision this through the configure script?

I'll be happy to look at it.  Can you point me to the right email
thread or location in the configure.ac file?

Atousa

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-10-30 22:22               ` Junio C Hamano
@ 2015-11-01  6:41                 ` Atousa Duprat
  2015-11-01 18:31                   ` Junio C Hamano
  0 siblings, 1 reply; 34+ messages in thread
From: Atousa Duprat @ 2015-11-01  6:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Atousa Pahlevan Duprat

>> +int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)
>> +{
>> +     size_t nr;
>> +     size_t total = 0;
>> +     char *cdata = (char*)data;

> I am not sure about the cast
> here, though.  Doesn't the function SHA1_Update() you are going to
> call in the body of the loop take "const void *" as its second
> parameter?  That's how openssl/sha1.h and block-sha1/sha1.h declare
> this function.

Indeed, the declaration needs a const void *; but I need to advance by
a specific number of bytes in each iteration of the loop.  Hence the
cast.

Atousa

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-11-01  6:32                 ` atousa.p
@ 2015-11-01  8:30                   ` Eric Sunshine
  2015-11-01 18:37                   ` Junio C Hamano
  1 sibling, 0 replies; 34+ messages in thread
From: Eric Sunshine @ 2015-11-01  8:30 UTC (permalink / raw)
  To: Atousa Pahlevan Duprat
  Cc: Git List, Junio C Hamano, rsbecker, Atousa Pahlevan Duprat

On Sun, Nov 1, 2015 at 1:32 AM,  <atousa.p@gmail.com> wrote:
> Some implementations of SHA_Updates have inherent limits
> on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
> to set the max chunk size supported, if required.  This is
> enabled for OSX CommonCrypto library and set to 1GiB.
>
> Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>
> ---
> diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
> new file mode 100644
> index 0000000..bf62b1b
> --- /dev/null
> +++ b/compat/sha1_chunked.c
> @@ -0,0 +1,21 @@
> +#include "cache.h"
> +
> +#ifdef SHA1_MAX_BLOCK_SIZE

This file is only compiled when SHA1_MAX_BLOCK_SIZE is defined, so
does this #ifdef serve a purpose?

> +int git_SHA1_Update(SHA_CTX *c, const void *data, size_t len)
> +{
> +       size_t nr;
> +       size_t total = 0;
> +       char *cdata = (char*)data;

Nit: This could be 'const char *'.

> +       while (len > 0) {
> +               nr = len;
> +               if (nr > SHA1_MAX_BLOCK_SIZE)
> +                       nr = SHA1_MAX_BLOCK_SIZE;
> +               SHA1_Update(c, cdata, nr);
> +               total += nr;
> +               cdata += nr;
> +               len -= nr;
> +       }
> +       return total;
> +}
> +#endif

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-11-01  6:41                 ` Atousa Duprat
@ 2015-11-01 18:31                   ` Junio C Hamano
  0 siblings, 0 replies; 34+ messages in thread
From: Junio C Hamano @ 2015-11-01 18:31 UTC (permalink / raw)
  To: Atousa Duprat; +Cc: git, Atousa Pahlevan Duprat

Atousa Duprat <atousa.p@gmail.com> writes:

> Indeed, the declaration needs a const void *; but I need to advance by
> a specific number of bytes in each iteration of the loop.  Hence the
> cast.

Ah, of course you are right.  Silly me.

Thanks.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-11-01  6:32                 ` atousa.p
  2015-11-01  8:30                   ` Eric Sunshine
@ 2015-11-01 18:37                   ` Junio C Hamano
  2015-11-02 20:52                     ` Atousa Duprat
  1 sibling, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2015-11-01 18:37 UTC (permalink / raw)
  To: atousa.p; +Cc: git, sunshine, rsbecker, Atousa Pahlevan Duprat

atousa.p@gmail.com writes:

> diff --git a/cache.h b/cache.h
> index 79066e5..ec84b16 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -14,7 +14,12 @@
>  #ifndef git_SHA_CTX
>  #define git_SHA_CTX	SHA_CTX
>  #define git_SHA1_Init	SHA1_Init
> -#define git_SHA1_Update	SHA1_Update
> +#ifdef SHA1_MAX_BLOCK_SIZE
> +extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);
> +#define git_SHA1_Update SHA1_Update_Chunked
> +#else
> +#define git_SHA1_Update SHA1_Update
> +#endif
>  #define git_SHA1_Final	SHA1_Final
>  #endif

Hmm, I admit that this mess is my creation, but unfortunately it
does not allow us to say:

	make SHA1_MAX_BLOCK_SIZE='1024L*1024L*1024L'

when using other SHA-1 implementations (e.g. blk_SHA1_Update()).

Ideas for cleaning it up, anybody?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-11-01 18:37                   ` Junio C Hamano
@ 2015-11-02 20:52                     ` Atousa Duprat
  2015-11-02 21:21                       ` Junio C Hamano
  0 siblings, 1 reply; 34+ messages in thread
From: Atousa Duprat @ 2015-11-02 20:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Sunshine, Randall Becker, Atousa Pahlevan Duprat

In the Makefile there is the following:

ifdef BLK_SHA1
        SHA1_HEADER = "block-sha1/sha1.h"
        LIB_OBJS += block-sha1/sha1.o
else
ifdef PPC_SHA1
        SHA1_HEADER = "ppc/sha1.h"
        LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
else
ifdef APPLE_COMMON_CRYPTO
        COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
        SHA1_HEADER = <CommonCrypto/CommonDigest.h>
        SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
else
        SHA1_HEADER = <openssl/sha.h>
        EXTLIBS += $(LIB_4_CRYPTO)
endif

which seems to imply that BLK_SHA1 and APPLE_COMMON_CRYPTO are
mutually exclusive?

On Sun, Nov 1, 2015 at 10:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
> atousa.p@gmail.com writes:
>
>> diff --git a/cache.h b/cache.h
>> index 79066e5..ec84b16 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -14,7 +14,12 @@
>>  #ifndef git_SHA_CTX
>>  #define git_SHA_CTX  SHA_CTX
>>  #define git_SHA1_Init        SHA1_Init
>> -#define git_SHA1_Update      SHA1_Update
>> +#ifdef SHA1_MAX_BLOCK_SIZE
>> +extern int SHA1_Update_Chunked(SHA_CTX *, const void *, size_t);
>> +#define git_SHA1_Update SHA1_Update_Chunked
>> +#else
>> +#define git_SHA1_Update SHA1_Update
>> +#endif
>>  #define git_SHA1_Final       SHA1_Final
>>  #endif
>
> Hmm, I admit that this mess is my creation, but unfortunately it
> does not allow us to say:
>
>         make SHA1_MAX_BLOCK_SIZE='1024L*1024L*1024L'
>
> when using other SHA-1 implementations (e.g. blk_SHA1_Update()).
>
> Ideas for cleaning it up, anybody?
>



-- 
Atousa Pahlevan, PhD
M.Math. University of Waterloo, Canada
Ph.D. Department of Computer Science, University of Victoria, Canada
Voice: 415-341-6206
Email: apahlevan@ieee.org
Website: www.apahlevan.org

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-11-02 20:52                     ` Atousa Duprat
@ 2015-11-02 21:21                       ` Junio C Hamano
  2015-11-03  6:58                         ` [PATCH 1/2] " atousa.p
  2015-11-04 17:09                         ` [PATCH] " Junio C Hamano
  0 siblings, 2 replies; 34+ messages in thread
From: Junio C Hamano @ 2015-11-02 21:21 UTC (permalink / raw)
  To: Atousa Duprat; +Cc: git, Eric Sunshine, Randall Becker, Atousa Pahlevan Duprat

Atousa Duprat <atousa.p@gmail.com> writes:

> On Sun, Nov 1, 2015 at 10:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Hmm, I admit that this mess is my creation, but unfortunately it
>> does not allow us to say:
>>
>>         make SHA1_MAX_BLOCK_SIZE='1024L*1024L*1024L'
>>
>> when using other SHA-1 implementations (e.g. blk_SHA1_Update()).
>>
>> Ideas for cleaning it up, anybody?
>>
> In the Makefile there is the following:
>
> ifdef BLK_SHA1
>         SHA1_HEADER = "block-sha1/sha1.h"
>         LIB_OBJS += block-sha1/sha1.o
> else
> ifdef PPC_SHA1
>         SHA1_HEADER = "ppc/sha1.h"
>         LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
> else
> ifdef APPLE_COMMON_CRYPTO
>         COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
>         SHA1_HEADER = <CommonCrypto/CommonDigest.h>
>         SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
> else
>         SHA1_HEADER = <openssl/sha.h>
>         EXTLIBS += $(LIB_4_CRYPTO)
> endif
>
> which seems to imply that BLK_SHA1 and APPLE_COMMON_CRYPTO are
> mutually exclusive?

Yes, you are correct that these two cannot be used at the same time.
In general (not limited to BLK_SHA1 and APPLE_COMMON_CRYPTO) you can
pick only _one_ underlying SHA-1 implementation to use with the
system.

If you use APPLE_COMMON_CRYPTO, you may have to use the Chunked
thing, because of APPLE_COMMON_CRYPTO implementation's limitation.

But the above two facts taken together does not have to imply that
you are forbidden from choosing to use Chunked thing if you are
using BLK_SHA1 or OpenSSL's SHA-1 implementation.  If only for
making sure that the Chunked wrapper passes compilation test, for
trying it out to see how well it works, or just for satisfying
curiosity, it would be nice if we allowed such a combination.

The original arrangement of macro was:

 * The user code uses git_SHA1_Update()

 * cache.h renames git_SHA1_Update() to refer to the underlying
   SHA1_Update() function, either from OpenSSL or AppleCommonCrypto,
   or block-sha1/sha1.h renames git_SHA1_Update() to refer to our
   implementation blk_SHA1_Update().

What we want with Chunked is:

 * The user code uses git_SHA1_Update(); we must not change this, as
   there are many existing calls.

 * We want git_SHA1_Update() to call the Chunked thing when
   SHA1_MAX_BLOCK_SIZE is set.

 * The Chunked thing must delegate the actual hashing to underlying
   SHA1_Update(), either from OpenSSL or AppleCommonCrypto.  If we
   are using BLK_SHA1, we want the Chunked thing to instead call
   blk_SHA1_Update().

I do not seem to be able to find a way to do this with the current
two-level indirection.  If we added another level, we can.

* In cache.h, define platform_SHA1_Update() to refer to
  SHA1_Update() from the platform (unless block-sha1/ is used).
  git_SHA1_Update() in the user code may directly call it, or it may
  go to the Chunked thing.

  #ifndef git_SHA1_CTX
  #define platform_SHA1_Update  SHA1_Update
  #endif

  #ifdef SHA1_MAX_BLOCK_SIZE
  #define git_SHA1_Update       git_SHA1_Update_Chunked
  #else
  #define git_SHA1_Update       platform_SHA1_Update
  #endif

* In block-sha1/sha1.h, redirect platform_SHA1_Update() to
  blk_SHA1_Update().

  #define platform_SHA1_Update  blk_SHA1_Update

* In compat/sha1_chunked.c, implement the Chunked thing in terms of
  the platform_SHA1_Update():

  git_SHA1_Update_Chunked(...)
  {
        ...
        while (...) {
                platform_SHA1_Update(...);
        }
  }


I am not sure if the above is worth it, but I suspect the damage is
localized enough that this may be OK.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 1/2] Limit the size of the data block passed to SHA1_Update()
  2015-11-02 21:21                       ` Junio C Hamano
@ 2015-11-03  6:58                         ` atousa.p
  2015-11-03 11:51                           ` Torsten Bögershausen
  2015-11-04 17:09                         ` [PATCH] " Junio C Hamano
  1 sibling, 1 reply; 34+ messages in thread
From: atousa.p @ 2015-11-03  6:58 UTC (permalink / raw)
  To: git, gitster; +Cc: Atousa Pahlevan Duprat

From: Atousa Pahlevan Duprat <apahlevan@ieee.org>

Some implementations of SHA_Updates have inherent limits
on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
to set the max chunk size supported, if required.  This is
enabled for OSX CommonCrypto library and set to 1GiB.

Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>
---
 Makefile                     | 16 +++++++++++++++-
 block-sha1/sha1.h            |  2 +-
 cache.h                      | 17 +++++++++++++----
 compat/apple-common-crypto.h |  4 ++++
 compat/sha1_chunked.c        | 19 +++++++++++++++++++
 5 files changed, 52 insertions(+), 6 deletions(-)
 create mode 100644 compat/sha1_chunked.c

diff --git a/Makefile b/Makefile
index 04c2231..1b098cc 100644
--- a/Makefile
+++ b/Makefile
@@ -136,11 +136,15 @@ all::
 # to provide your own OpenSSL library, for example from MacPorts.
 #
 # Define BLK_SHA1 environment variable to make use of the bundled
-# optimized C SHA1 routine.
+# optimized C SHA1 routine.  This implies NO_APPLE_COMMON_CRYPTO.
 #
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
+#
 # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
 #
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
@@ -986,6 +990,10 @@ ifeq (no,$(USE_PARENS_AROUND_GETTEXT_N))
 endif
 endif
 
+ifdef BLK_SHA1
+	NO_APPLE_COMMON_CRYPTO=1
+endif
+
 ifeq ($(uname_S),Darwin)
 	ifndef NO_FINK
 		ifeq ($(shell test -d /sw/lib && echo y),y)
@@ -1346,6 +1354,8 @@ else
 ifdef APPLE_COMMON_CRYPTO
 	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
 	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
+	# Apple CommonCrypto requires chunking
+	SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
 else
 	SHA1_HEADER = <openssl/sha.h>
 	EXTLIBS += $(LIB_4_CRYPTO)
@@ -1353,6 +1363,10 @@ endif
 endif
 endif
 
+ifdef SHA1_MAX_BLOCK_SIZE
+	LIB_OBJS += compat/sha1_chunked.o
+	BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
+endif
 ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif
diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
index b864df6..d085412 100644
--- a/block-sha1/sha1.h
+++ b/block-sha1/sha1.h
@@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx);
 
 #define git_SHA_CTX	blk_SHA_CTX
 #define git_SHA1_Init	blk_SHA1_Init
-#define git_SHA1_Update	blk_SHA1_Update
+#define platform_SHA1_Update	blk_SHA1_Update
 #define git_SHA1_Final	blk_SHA1_Final
diff --git a/cache.h b/cache.h
index 79066e5..a501652 100644
--- a/cache.h
+++ b/cache.h
@@ -10,12 +10,21 @@
 #include "trace.h"
 #include "string-list.h"
 
+// platform's underlying implementation of SHA1
 #include SHA1_HEADER
 #ifndef git_SHA_CTX
-#define git_SHA_CTX	SHA_CTX
-#define git_SHA1_Init	SHA1_Init
-#define git_SHA1_Update	SHA1_Update
-#define git_SHA1_Final	SHA1_Final
+#define git_SHA_CTX		SHA_CTX
+#define git_SHA1_Init		SHA1_Init
+#define platform_SHA1_Update	SHA1_Update
+#define git_SHA1_Final		SHA1_Final
+#endif
+
+// choose whether chunked implementation or not
+#ifdef SHA1_MAX_BLOCK_SIZE
+int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);
+#define git_SHA1_Update       git_SHA1_Update_Chunked
+#else
+#define git_SHA1_Update       platform_SHA1_Update
 #endif
 
 #include <zlib.h>
diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
index c8b9b0e..d3fb264 100644
--- a/compat/apple-common-crypto.h
+++ b/compat/apple-common-crypto.h
@@ -16,6 +16,10 @@
 #undef TYPE_BOOL
 #endif
 
+#ifndef SHA1_MAX_BLOCK_SIZE
+#error Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE
+#endif
+
 #ifdef APPLE_LION_OR_NEWER
 #define git_CC_error_check(pattern, err) \
 	do { \
diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
new file mode 100644
index 0000000..61f67de
--- /dev/null
+++ b/compat/sha1_chunked.c
@@ -0,0 +1,19 @@
+#include "cache.h"
+
+int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)
+{
+	size_t nr;
+	size_t total = 0;
+	const char *cdata = (const char*)data;
+
+	while (len > 0) {
+		nr = len;
+		if (nr > SHA1_MAX_BLOCK_SIZE)
+			nr = SHA1_MAX_BLOCK_SIZE;
+		platform_SHA1_Update(c, cdata, nr);
+		total += nr;
+		cdata += nr;
+		len -= nr;
+	}
+	return total;
+}
-- 
2.4.9 (Apple Git-60)

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/2] Limit the size of the data block passed to SHA1_Update()
  2015-11-03  6:58                         ` [PATCH 1/2] " atousa.p
@ 2015-11-03 11:51                           ` Torsten Bögershausen
  2015-11-04  4:24                             ` [PATCH] " atousa.p
  2015-11-04  4:27                             ` [PATCH 1/2] Limit the size of the data block passed to SHA1_Update() Atousa Duprat
  0 siblings, 2 replies; 34+ messages in thread
From: Torsten Bögershausen @ 2015-11-03 11:51 UTC (permalink / raw)
  To: atousa.p, git, gitster; +Cc: Atousa Pahlevan Duprat

On 11/03/2015 07:58 AM, atousa.p@gmail.com wrote:
> From: Atousa Pahlevan Duprat <apahlevan@ieee.org>
Minor comments inline
> diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
> index b864df6..d085412 100644
> --- a/block-sha1/sha1.h
> +++ b/block-sha1/sha1.h
> @@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx);
>   
>   #define git_SHA_CTX	blk_SHA_CTX
>   #define git_SHA1_Init	blk_SHA1_Init
> -#define git_SHA1_Update	blk_SHA1_Update
> +#define platform_SHA1_Update	blk_SHA1_Update
>   #define git_SHA1_Final	blk_SHA1_Final
> diff --git a/cache.h b/cache.h
> index 79066e5..a501652 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -10,12 +10,21 @@
>   #include "trace.h"
>   #include "string-list.h"
>   
> +// platform's underlying implementation of SHA1
Please use /* */ for comments
>   #include SHA1_HEADER
>   #ifndef git_SHA_CTX
> -#define git_SHA_CTX	SHA_CTX
> -#define git_SHA1_Init	SHA1_Init
> -#define git_SHA1_Update	SHA1_Update
> -#define git_SHA1_Final	SHA1_Final
> +#define git_SHA_CTX		SHA_CTX
> +#define git_SHA1_Init		SHA1_Init
> +#define platform_SHA1_Update	SHA1_Update
> +#define git_SHA1_Final		SHA1_Final
> +#endif
> +
> +// choose whether chunked implementation or not
> +#ifdef SHA1_MAX_BLOCK_SIZE
> +int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);
> +#define git_SHA1_Update       git_SHA1_Update_Chunked
> +#else
> +#define git_SHA1_Update       platform_SHA1_Update
>   #endif
>   
>   #include <zlib.h>
> diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
> index c8b9b0e..d3fb264 100644
> --- a/compat/apple-common-crypto.h
> +++ b/compat/apple-common-crypto.h
> @@ -16,6 +16,10 @@
>   #undef TYPE_BOOL
>   #endif
>   
> +#ifndef SHA1_MAX_BLOCK_SIZE
> +#error Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE
> +#endif
> +
>   #ifdef APPLE_LION_OR_NEWER
>   #define git_CC_error_check(pattern, err) \
>   	do { \
> diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
> new file mode 100644
> index 0000000..61f67de
> --- /dev/null
> +++ b/compat/sha1_chunked.c
> @@ -0,0 +1,19 @@
> +#include "cache.h"
> +
> +int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)
> +{
> +	size_t nr;
> +	size_t total = 0;
> +	const char *cdata = (const char*)data;
> +
> +	while (len > 0) {
size_t is unsigned, isn't it ?
Better to use  "while (len) {"
> +		nr = len;
> +		if (nr > SHA1_MAX_BLOCK_SIZE)
> +			nr = SHA1_MAX_BLOCK_SIZE;
> +		platform_SHA1_Update(c, cdata, nr);
> +		total += nr;
> +		cdata += nr;
> +		len -= nr;
> +	}
> +	return total;
> +}

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-11-03 11:51                           ` Torsten Bögershausen
@ 2015-11-04  4:24                             ` atousa.p
  2015-11-04 19:51                               ` Eric Sunshine
  2015-11-04  4:27                             ` [PATCH 1/2] Limit the size of the data block passed to SHA1_Update() Atousa Duprat
  1 sibling, 1 reply; 34+ messages in thread
From: atousa.p @ 2015-11-04  4:24 UTC (permalink / raw)
  To: git, gitster; +Cc: Atousa Pahlevan Duprat

From: Atousa Pahlevan Duprat <apahlevan@ieee.org>

Some implementations of SHA_Updates have inherent limits
on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
to set the max chunk size supported, if required.  This is
enabled for OSX CommonCrypto library and set to 1GiB.

Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>
---
 Makefile                     | 16 +++++++++++++++-
 block-sha1/sha1.h            |  2 +-
 cache.h                      | 17 +++++++++++++----
 compat/apple-common-crypto.h |  4 ++++
 compat/sha1_chunked.c        | 19 +++++++++++++++++++
 5 files changed, 52 insertions(+), 6 deletions(-)
 create mode 100644 compat/sha1_chunked.c

diff --git a/Makefile b/Makefile
index 04c2231..1b098cc 100644
--- a/Makefile
+++ b/Makefile
@@ -136,11 +136,15 @@ all::
 # to provide your own OpenSSL library, for example from MacPorts.
 #
 # Define BLK_SHA1 environment variable to make use of the bundled
-# optimized C SHA1 routine.
+# optimized C SHA1 routine.  This implies NO_APPLE_COMMON_CRYPTO.
 #
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
+# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
+# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
+# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
+#
 # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
 #
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
@@ -986,6 +990,10 @@ ifeq (no,$(USE_PARENS_AROUND_GETTEXT_N))
 endif
 endif
 
+ifdef BLK_SHA1
+	NO_APPLE_COMMON_CRYPTO=1
+endif
+
 ifeq ($(uname_S),Darwin)
 	ifndef NO_FINK
 		ifeq ($(shell test -d /sw/lib && echo y),y)
@@ -1346,6 +1354,8 @@ else
 ifdef APPLE_COMMON_CRYPTO
 	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
 	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
+	# Apple CommonCrypto requires chunking
+	SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
 else
 	SHA1_HEADER = <openssl/sha.h>
 	EXTLIBS += $(LIB_4_CRYPTO)
@@ -1353,6 +1363,10 @@ endif
 endif
 endif
 
+ifdef SHA1_MAX_BLOCK_SIZE
+	LIB_OBJS += compat/sha1_chunked.o
+	BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
+endif
 ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif
diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
index b864df6..d085412 100644
--- a/block-sha1/sha1.h
+++ b/block-sha1/sha1.h
@@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx);
 
 #define git_SHA_CTX	blk_SHA_CTX
 #define git_SHA1_Init	blk_SHA1_Init
-#define git_SHA1_Update	blk_SHA1_Update
+#define platform_SHA1_Update	blk_SHA1_Update
 #define git_SHA1_Final	blk_SHA1_Final
diff --git a/cache.h b/cache.h
index 79066e5..e345e38 100644
--- a/cache.h
+++ b/cache.h
@@ -10,12 +10,21 @@
 #include "trace.h"
 #include "string-list.h"
 
+/* platform's underlying implementation of SHA1 */
 #include SHA1_HEADER
 #ifndef git_SHA_CTX
-#define git_SHA_CTX	SHA_CTX
-#define git_SHA1_Init	SHA1_Init
-#define git_SHA1_Update	SHA1_Update
-#define git_SHA1_Final	SHA1_Final
+#define git_SHA_CTX		SHA_CTX
+#define git_SHA1_Init		SHA1_Init
+#define platform_SHA1_Update	SHA1_Update
+#define git_SHA1_Final		SHA1_Final
+#endif
+
+/* choose chunked implementation or not */
+#ifdef SHA1_MAX_BLOCK_SIZE
+int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);
+#define git_SHA1_Update       git_SHA1_Update_Chunked
+#else
+#define git_SHA1_Update       platform_SHA1_Update
 #endif
 
 #include <zlib.h>
diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
index c8b9b0e..d3fb264 100644
--- a/compat/apple-common-crypto.h
+++ b/compat/apple-common-crypto.h
@@ -16,6 +16,10 @@
 #undef TYPE_BOOL
 #endif
 
+#ifndef SHA1_MAX_BLOCK_SIZE
+#error Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE
+#endif
+
 #ifdef APPLE_LION_OR_NEWER
 #define git_CC_error_check(pattern, err) \
 	do { \
diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
new file mode 100644
index 0000000..6d0062b
--- /dev/null
+++ b/compat/sha1_chunked.c
@@ -0,0 +1,19 @@
+#include "cache.h"
+
+int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)
+{
+	size_t nr;
+	size_t total = 0;
+	const char *cdata = (const char*)data;
+
+	while (len) {
+		nr = len;
+		if (nr > SHA1_MAX_BLOCK_SIZE)
+			nr = SHA1_MAX_BLOCK_SIZE;
+		platform_SHA1_Update(c, cdata, nr);
+		total += nr;
+		cdata += nr;
+		len -= nr;
+	}
+	return total;
+}
-- 
2.4.9 (Apple Git-60)

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/2] Limit the size of the data block passed to SHA1_Update()
  2015-11-03 11:51                           ` Torsten Bögershausen
  2015-11-04  4:24                             ` [PATCH] " atousa.p
@ 2015-11-04  4:27                             ` Atousa Duprat
  1 sibling, 0 replies; 34+ messages in thread
From: Atousa Duprat @ 2015-11-04  4:27 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git, Junio C Hamano, Atousa Pahlevan Duprat

Thank you for the feedback. The patch is updated as suggested.


On Tue, Nov 3, 2015 at 3:51 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 11/03/2015 07:58 AM, atousa.p@gmail.com wrote:
>>
>> From: Atousa Pahlevan Duprat <apahlevan@ieee.org>
>
> Minor comments inline
>>
>> diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
>> index b864df6..d085412 100644
>> --- a/block-sha1/sha1.h
>> +++ b/block-sha1/sha1.h
>> @@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20],
>> blk_SHA_CTX *ctx);
>>     #define git_SHA_CTX blk_SHA_CTX
>>   #define git_SHA1_Init blk_SHA1_Init
>> -#define git_SHA1_Update        blk_SHA1_Update
>> +#define platform_SHA1_Update   blk_SHA1_Update
>>   #define git_SHA1_Final        blk_SHA1_Final
>> diff --git a/cache.h b/cache.h
>> index 79066e5..a501652 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -10,12 +10,21 @@
>>   #include "trace.h"
>>   #include "string-list.h"
>>   +// platform's underlying implementation of SHA1
>
> Please use /* */ for comments
>
>>   #include SHA1_HEADER
>>   #ifndef git_SHA_CTX
>> -#define git_SHA_CTX    SHA_CTX
>> -#define git_SHA1_Init  SHA1_Init
>> -#define git_SHA1_Update        SHA1_Update
>> -#define git_SHA1_Final SHA1_Final
>> +#define git_SHA_CTX            SHA_CTX
>> +#define git_SHA1_Init          SHA1_Init
>> +#define platform_SHA1_Update   SHA1_Update
>> +#define git_SHA1_Final         SHA1_Final
>> +#endif
>> +
>> +// choose whether chunked implementation or not
>> +#ifdef SHA1_MAX_BLOCK_SIZE
>> +int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);
>> +#define git_SHA1_Update       git_SHA1_Update_Chunked
>> +#else
>> +#define git_SHA1_Update       platform_SHA1_Update
>>   #endif
>>     #include <zlib.h>
>> diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
>> index c8b9b0e..d3fb264 100644
>> --- a/compat/apple-common-crypto.h
>> +++ b/compat/apple-common-crypto.h
>> @@ -16,6 +16,10 @@
>>   #undef TYPE_BOOL
>>   #endif
>>   +#ifndef SHA1_MAX_BLOCK_SIZE
>> +#error Using Apple Common Crypto library requires setting
>> SHA1_MAX_BLOCK_SIZE
>> +#endif
>> +
>>   #ifdef APPLE_LION_OR_NEWER
>>   #define git_CC_error_check(pattern, err) \
>>         do { \
>> diff --git a/compat/sha1_chunked.c b/compat/sha1_chunked.c
>> new file mode 100644
>> index 0000000..61f67de
>> --- /dev/null
>> +++ b/compat/sha1_chunked.c
>> @@ -0,0 +1,19 @@
>> +#include "cache.h"
>> +
>> +int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len)
>> +{
>> +       size_t nr;
>> +       size_t total = 0;
>> +       const char *cdata = (const char*)data;
>> +
>> +       while (len > 0) {
>
> size_t is unsigned, isn't it ?
> Better to use  "while (len) {"
>
>> +               nr = len;
>> +               if (nr > SHA1_MAX_BLOCK_SIZE)
>> +                       nr = SHA1_MAX_BLOCK_SIZE;
>> +               platform_SHA1_Update(c, cdata, nr);
>> +               total += nr;
>> +               cdata += nr;
>> +               len -= nr;
>> +       }
>> +       return total;
>> +}
>
>



-- 
Atousa Pahlevan, PhD
M.Math. University of Waterloo, Canada
Ph.D. Department of Computer Science, University of Victoria, Canada
Voice: 415-341-6206
Email: apahlevan@ieee.org
Website: www.apahlevan.org

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-11-02 21:21                       ` Junio C Hamano
  2015-11-03  6:58                         ` [PATCH 1/2] " atousa.p
@ 2015-11-04 17:09                         ` Junio C Hamano
  1 sibling, 0 replies; 34+ messages in thread
From: Junio C Hamano @ 2015-11-04 17:09 UTC (permalink / raw)
  To: Atousa Duprat; +Cc: git, Eric Sunshine, Randall Becker, Atousa Pahlevan Duprat

Junio C Hamano <gitster@pobox.com> writes:

>> ifdef BLK_SHA1
>>         SHA1_HEADER = "block-sha1/sha1.h"
>>         LIB_OBJS += block-sha1/sha1.o
>> else
>> ifdef PPC_SHA1
>>         SHA1_HEADER = "ppc/sha1.h"
>>         LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
>> else
>> ifdef APPLE_COMMON_CRYPTO
>>         COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
>>         SHA1_HEADER = <CommonCrypto/CommonDigest.h>
>>         SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
>> else
>>         SHA1_HEADER = <openssl/sha.h>
>>         EXTLIBS += $(LIB_4_CRYPTO)
>> endif
>>
>> which seems to imply that BLK_SHA1 and APPLE_COMMON_CRYPTO are
>> mutually exclusive?
>
> Yes, you are correct that these two cannot be used at the same time.
> In general (not limited to BLK_SHA1 and APPLE_COMMON_CRYPTO) you can
> pick only _one_ underlying SHA-1 implementation to use with the
> system.

Our "seems to imply" above is a faulty reading.  The four lines I
wrote are not incorrect per-se, but this exchange was misleading.

When BLK_SHA1 is defined, the above fragment from the Makefile is
only saying "CommonCrypto may or may not be used for any other
purposes, but for SHA-1 hashing, I'll use our own block-sha1/
implementation."  It does not say anything about how the system
favours CommonCrypto over OpenSSL with APPLE_COMMON_CRYPTO.

And it is legit to define both APPLE_COMMON_CRYPTO and BLK_SHA1; the
resulting build would still use SSL-related functions what other
people may use from OpenSSL from CommonCrypto.  Filipe Cabecinhas
pointed out that such a configuration works (and solves the issue
that triggered this thread) very early in the thread.

I haven't looked carefully at the latest version of your patch, but
I just wanted to make sure that I didn't mislead you to add an
unnecessary "we check if both APPLE_COMMON_CRYPTO and BLK_SHA1 are
defined and error out because they are incompatible" check.  Sorry
for an earlier message that may have been confusing.

Thanks.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
  2015-11-04  4:24                             ` [PATCH] " atousa.p
@ 2015-11-04 19:51                               ` Eric Sunshine
  2015-11-05  6:38                                 ` [PATCH v4 1/3] Provide another level of abstraction for the SHA1 utilities atousa.p
                                                   ` (2 more replies)
  0 siblings, 3 replies; 34+ messages in thread
From: Eric Sunshine @ 2015-11-04 19:51 UTC (permalink / raw)
  To: atousa.p; +Cc: git, gitster, Atousa Pahlevan Duprat

On Tuesday, November 3, 2015, <atousa.p@gmail.com> wrote:
> [PATCH] Limit the size of the data block passed to SHA1_Update()

As an aid for reviewers, please include the version number of the
patch, such as [PATCH vN] where "N" is the revision. format-patch's -v
option can help automate this.

> Some implementations of SHA_Updates have inherent limits
> on the max chunk size. SHA1_MAX_BLOCK_SIZE can be defined
> to set the max chunk size supported, if required.  This is
> enabled for OSX CommonCrypto library and set to 1GiB.
>
> Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>
> ---

It is helpful to reviewers if you can describe here, below the "---"
line, how this version of the patch differs from the previous one.
More below...

> diff --git a/Makefile b/Makefile
> @@ -136,11 +136,15 @@ all::
>  # to provide your own OpenSSL library, for example from MacPorts.
>  #
>  # Define BLK_SHA1 environment variable to make use of the bundled
> -# optimized C SHA1 routine.
> +# optimized C SHA1 routine.  This implies NO_APPLE_COMMON_CRYPTO.
>  #
>  # Define PPC_SHA1 environment variable when running make to make use of
>  # a bundled SHA1 routine optimized for PowerPC.
>  #
> +# Define SHA1_MAX_BLOCK_SIZE if your SSH1_Update() implementation can
> +# hash only a limited amount of data in one call (e.g. APPLE_COMMON_CRYPTO
> +# may want 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined).
> +#
>  # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
>  #
>  # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
> @@ -986,6 +990,10 @@ ifeq (no,$(USE_PARENS_AROUND_GETTEXT_N))
>  endif
>  endif
>
> +ifdef BLK_SHA1
> +       NO_APPLE_COMMON_CRYPTO=1
> +endif

I think Junio already mentioned[1] that this change (and its
corresponding documentation update above) likely is undesirable, but I
just wanted to mention that you would typically want to split such a
change out to a separate patch since it's unrelated to the overall
intention of the current patch. More below...

[1]: http://article.gmane.org/gmane.comp.version-control.git/280859

>  ifeq ($(uname_S),Darwin)
>         ifndef NO_FINK
>                 ifeq ($(shell test -d /sw/lib && echo y),y)
> @@ -1346,6 +1354,8 @@ else
>  ifdef APPLE_COMMON_CRYPTO
>         COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
>         SHA1_HEADER = <CommonCrypto/CommonDigest.h>
> +       # Apple CommonCrypto requires chunking
> +       SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
>  else
>         SHA1_HEADER = <openssl/sha.h>
>         EXTLIBS += $(LIB_4_CRYPTO)
> @@ -1353,6 +1363,10 @@ endif
>  endif
>  endif
>
> +ifdef SHA1_MAX_BLOCK_SIZE
> +       LIB_OBJS += compat/sha1_chunked.o
> +       BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
> +endif
> diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
> @@ -18,5 +18,5 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx);
>
>  #define git_SHA_CTX    blk_SHA_CTX
>  #define git_SHA1_Init  blk_SHA1_Init
> -#define git_SHA1_Update        blk_SHA1_Update
> +#define platform_SHA1_Update   blk_SHA1_Update
>  #define git_SHA1_Final blk_SHA1_Final
> diff --git a/cache.h b/cache.h
> index 79066e5..e345e38 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -10,12 +10,21 @@
>  #include "trace.h"
>  #include "string-list.h"
>
> +/* platform's underlying implementation of SHA1 */
>  #include SHA1_HEADER
>  #ifndef git_SHA_CTX
> -#define git_SHA_CTX    SHA_CTX
> -#define git_SHA1_Init  SHA1_Init
> -#define git_SHA1_Update        SHA1_Update
> -#define git_SHA1_Final SHA1_Final
> +#define git_SHA_CTX            SHA_CTX
> +#define git_SHA1_Init          SHA1_Init
> +#define platform_SHA1_Update   SHA1_Update
> +#define git_SHA1_Final         SHA1_Final
> +#endif

It's a bit ugly that this special-cases only "update" with the
"platform_" abstraction. It _might_ be preferable to generalize this
for all the SHA1 operations. If so, that's something you'd want to do
as a separate preparatory patch specifically aimed at adding this new
abstraction layer.

(In fact, even if you decide only to special-case "update", that still
might deserve a separate preparatory patch since it's a conceptually
distinct change from the overall focus of this patch, and would make
this patch less noisy, thus easier to review.)

> +/* choose chunked implementation or not */
> +#ifdef SHA1_MAX_BLOCK_SIZE
> +int git_SHA1_Update_Chunked(SHA_CTX *c, const void *data, size_t len);
> +#define git_SHA1_Update       git_SHA1_Update_Chunked
> +#else
> +#define git_SHA1_Update       platform_SHA1_Update
>  #endif

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v4 1/3] Provide another level of abstraction for the SHA1 utilities.
  2015-11-04 19:51                               ` Eric Sunshine
@ 2015-11-05  6:38                                 ` atousa.p
  2015-11-05 18:29                                   ` Junio C Hamano
  2015-11-05  6:38                                 ` [PATCH v4 2/3] Limit the size of the data block passed to SHA1_Update() atousa.p
  2015-11-05  6:38                                 ` [PATCH v4 3/3] Move all the SHA1 implementations into one directory atousa.p
  2 siblings, 1 reply; 34+ messages in thread
From: atousa.p @ 2015-11-05  6:38 UTC (permalink / raw)
  To: git; +Cc: Atousa Pahlevan Duprat

From: Atousa Pahlevan Duprat <apahlevan@ieee.org>

The git source uses git_SHA1_Update() and friends to call
into the code that computes the hashes.  This is can then be
mapped directly to an implementation that computes the hash,
such as platform_SHA1_Update(); or as we will do in a subsequent
patch, it can be mapped to something more complex that will in turn call
into the platform's SHA implementation.

Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>
---
 cache.h | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/cache.h b/cache.h
index a9aaa03..a934a2e 100644
--- a/cache.h
+++ b/cache.h
@@ -12,10 +12,21 @@
 
 #include SHA1_HEADER
 #ifndef git_SHA_CTX
-#define git_SHA_CTX	SHA_CTX
-#define git_SHA1_Init	SHA1_Init
-#define git_SHA1_Update	SHA1_Update
-#define git_SHA1_Final	SHA1_Final
+
+/* platform's underlying implementation of SHA1, could be OpenSSL,
+   blk_SHA, Apple CommonCrypto, etc...  */
+#define platform_SHA_CTX	SHA_CTX
+#define platform_SHA1_Init	SHA1_Init
+#define platform_SHA1_Update	SHA1_Update
+#define platform_SHA1_Final    	SHA1_Final
+
+/* git may call platform's underlying implementation of SHA1 directly,
+   or may call it through a wrapper */
+#define git_SHA_CTX		platform_SHA_CTX
+#define git_SHA1_Init		platform_SHA1_Init
+#define git_SHA1_Update		platform_SHA1_Update
+#define git_SHA1_Final		platform_SHA1_Final
+
 #endif
 
 #include <zlib.h>
-- 
2.4.9 (Apple Git-60)

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v4 2/3] Limit the size of the data block passed to SHA1_Update()
  2015-11-04 19:51                               ` Eric Sunshine
  2015-11-05  6:38                                 ` [PATCH v4 1/3] Provide another level of abstraction for the SHA1 utilities atousa.p
@ 2015-11-05  6:38                                 ` atousa.p
  2015-11-05 18:29                                   ` Junio C Hamano
  2015-11-05  6:38                                 ` [PATCH v4 3/3] Move all the SHA1 implementations into one directory atousa.p
  2 siblings, 1 reply; 34+ messages in thread
From: atousa.p @ 2015-11-05  6:38 UTC (permalink / raw)
  To: git; +Cc: Atousa Pahlevan Duprat

From: Atousa Pahlevan Duprat <apahlevan@ieee.org>

Use the previous commit's abstraction mechanism for SHA1 to
support a chunked implementation of SHA1_Update() which limits
the amount of data in the chunk passed to SHA1_Update().

This is enabled by using the SHA1_MAX_BLOCK_SIZE envvar to
specify chunk size.  When using Apple's CommonCrypto library
this is enable and set to 1GiB.

Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>
---
 Makefile                     | 13 +++++++++++++
 cache.h                      | 17 +++++++++++++++--
 compat/apple-common-crypto.h |  4 ++++
 compat/sha1-chunked.c        | 19 +++++++++++++++++++
 compat/sha1-chunked.h        |  2 ++
 5 files changed, 53 insertions(+), 2 deletions(-)
 create mode 100644 compat/sha1-chunked.c
 create mode 100644 compat/sha1-chunked.h

diff --git a/Makefile b/Makefile
index 04c2231..6a4ca59 100644
--- a/Makefile
+++ b/Makefile
@@ -141,6 +141,10 @@ all::
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
+# Define SHA1_MAX_BLOCK_SIZE to limit the amount of data that will be hashed
+# in one call to the platform's SHA1_Update(). e.g. APPLE_COMMON_CRYPTO
+# wants 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined.
+#
 # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
 #
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
@@ -1335,6 +1339,11 @@ ifdef NO_POSIX_GOODIES
 	BASIC_CFLAGS += -DNO_POSIX_GOODIES
 endif
 
+ifdef APPLE_COMMON_CRYPTO
+	# Apple CommonCrypto requires chunking
+	SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
+endif
+
 ifdef BLK_SHA1
 	SHA1_HEADER = "block-sha1/sha1.h"
 	LIB_OBJS += block-sha1/sha1.o
@@ -1353,6 +1362,10 @@ endif
 endif
 endif
 
+ifdef SHA1_MAX_BLOCK_SIZE
+	LIB_OBJS += compat/sha1-chunked.o
+	BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
+endif
 ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif
diff --git a/cache.h b/cache.h
index a934a2e..182ac62 100644
--- a/cache.h
+++ b/cache.h
@@ -11,7 +11,7 @@
 #include "string-list.h"
 
 #include SHA1_HEADER
-#ifndef git_SHA_CTX
+#ifndef platform_SHA_CTX
 
 /* platform's underlying implementation of SHA1, could be OpenSSL,
    blk_SHA, Apple CommonCrypto, etc...  */
@@ -20,15 +20,28 @@
 #define platform_SHA1_Update	SHA1_Update
 #define platform_SHA1_Final    	SHA1_Final
 
+#endif
+
 /* git may call platform's underlying implementation of SHA1 directly,
    or may call it through a wrapper */
+
+#ifndef git_SHA_CTX
+
+#ifdef SHA1_MAX_BLOCK_SIZE
+#include "compat/sha1-chunked.h"
+#define git_SHA_CTX		platform_SHA_CTX
+#define git_SHA1_Init		platform_SHA1_Init
+#define git_SHA1_Update		git_SHA1_Update_Chunked
+#define git_SHA1_Final		platform_SHA1_Final
+#else
 #define git_SHA_CTX		platform_SHA_CTX
 #define git_SHA1_Init		platform_SHA1_Init
 #define git_SHA1_Update		platform_SHA1_Update
 #define git_SHA1_Final		platform_SHA1_Final
-
 #endif
 
+#endif 
+
 #include <zlib.h>
 typedef struct git_zstream {
 	z_stream z;
diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
index c8b9b0e..d3fb264 100644
--- a/compat/apple-common-crypto.h
+++ b/compat/apple-common-crypto.h
@@ -16,6 +16,10 @@
 #undef TYPE_BOOL
 #endif
 
+#ifndef SHA1_MAX_BLOCK_SIZE
+#error Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE
+#endif
+
 #ifdef APPLE_LION_OR_NEWER
 #define git_CC_error_check(pattern, err) \
 	do { \
diff --git a/compat/sha1-chunked.c b/compat/sha1-chunked.c
new file mode 100644
index 0000000..6adfcfd
--- /dev/null
+++ b/compat/sha1-chunked.c
@@ -0,0 +1,19 @@
+#include "cache.h"
+
+int git_SHA1_Update_Chunked(platform_SHA_CTX *c, const void *data, size_t len)
+{
+	size_t nr;
+	size_t total = 0;
+	const char *cdata = (const char*)data;
+
+	while (len) {
+		nr = len;
+		if (nr > SHA1_MAX_BLOCK_SIZE)
+			nr = SHA1_MAX_BLOCK_SIZE;
+		platform_SHA1_Update(c, cdata, nr);
+		total += nr;
+		cdata += nr;
+		len -= nr;
+	}
+	return total;
+}
diff --git a/compat/sha1-chunked.h b/compat/sha1-chunked.h
new file mode 100644
index 0000000..7b2df28
--- /dev/null
+++ b/compat/sha1-chunked.h
@@ -0,0 +1,2 @@
+
+int git_SHA1_Update_Chunked(platform_SHA_CTX *c, const void *data, size_t len);
-- 
2.4.9 (Apple Git-60)

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v4 3/3] Move all the SHA1 implementations into one directory
  2015-11-04 19:51                               ` Eric Sunshine
  2015-11-05  6:38                                 ` [PATCH v4 1/3] Provide another level of abstraction for the SHA1 utilities atousa.p
  2015-11-05  6:38                                 ` [PATCH v4 2/3] Limit the size of the data block passed to SHA1_Update() atousa.p
@ 2015-11-05  6:38                                 ` atousa.p
  2015-11-05 18:29                                   ` Junio C Hamano
  2 siblings, 1 reply; 34+ messages in thread
From: atousa.p @ 2015-11-05  6:38 UTC (permalink / raw)
  To: git; +Cc: Atousa Pahlevan Duprat

From: Atousa Pahlevan Duprat <apahlevan@ieee.org>

The various SHA1 implementations were spread around in 3 directories.
This makes it easier to understand what implementations are
available at a glance.

Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>
---
 Makefile              |  10 +-
 block-sha1/sha1.c     | 251 --------------------------------------------------
 block-sha1/sha1.h     |  22 -----
 cache.h               |   2 +-
 compat/sha1-chunked.c |  19 ----
 compat/sha1-chunked.h |   2 -
 ppc/sha1.c            |  72 ---------------
 ppc/sha1.h            |  25 -----
 ppc/sha1ppc.S         | 224 --------------------------------------------
 sha1/blk-sha1.c       | 251 ++++++++++++++++++++++++++++++++++++++++++++++++++
 sha1/blk-sha1.h       |  22 +++++
 sha1/chk-sha1.c       |  19 ++++
 sha1/chk-sha1.h       |   2 +
 sha1/ppc-sha1.c       |  72 +++++++++++++++
 sha1/ppc-sha1.h       |  25 +++++
 sha1/ppc-sha1asm.S    | 224 ++++++++++++++++++++++++++++++++++++++++++++
 16 files changed, 621 insertions(+), 621 deletions(-)
 delete mode 100644 block-sha1/sha1.c
 delete mode 100644 block-sha1/sha1.h
 delete mode 100644 compat/sha1-chunked.c
 delete mode 100644 compat/sha1-chunked.h
 delete mode 100644 ppc/sha1.c
 delete mode 100644 ppc/sha1.h
 delete mode 100644 ppc/sha1ppc.S
 create mode 100644 sha1/blk-sha1.c
 create mode 100644 sha1/blk-sha1.h
 create mode 100644 sha1/chk-sha1.c
 create mode 100644 sha1/chk-sha1.h
 create mode 100644 sha1/ppc-sha1.c
 create mode 100644 sha1/ppc-sha1.h
 create mode 100644 sha1/ppc-sha1asm.S

diff --git a/Makefile b/Makefile
index 6a4ca59..94f74d7 100644
--- a/Makefile
+++ b/Makefile
@@ -1345,12 +1345,12 @@ ifdef APPLE_COMMON_CRYPTO
 endif
 
 ifdef BLK_SHA1
-	SHA1_HEADER = "block-sha1/sha1.h"
-	LIB_OBJS += block-sha1/sha1.o
+	SHA1_HEADER = "sha1/blk-sha1.h"
+	LIB_OBJS += sha1/blk-sha1.o
 else
 ifdef PPC_SHA1
-	SHA1_HEADER = "ppc/sha1.h"
-	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
+	SHA1_HEADER = "sha1/ppc-sha1.h"
+	LIB_OBJS += sha1/ppc-sha1.o sha1/ppc-sha1asm.o
 else
 ifdef APPLE_COMMON_CRYPTO
 	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
@@ -1363,7 +1363,7 @@ endif
 endif
 
 ifdef SHA1_MAX_BLOCK_SIZE
-	LIB_OBJS += compat/sha1-chunked.o
+	LIB_OBJS += sha1/chk-sha1.o
 	BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
 endif
 ifdef NO_PERL_MAKEMAKER
diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
deleted file mode 100644
index 22b125c..0000000
--- a/block-sha1/sha1.c
+++ /dev/null
@@ -1,251 +0,0 @@
-/*
- * SHA1 routine optimized to do word accesses rather than byte accesses,
- * and to avoid unnecessary copies into the context array.
- *
- * This was initially based on the Mozilla SHA1 implementation, although
- * none of the original Mozilla code remains.
- */
-
-/* this is only to get definitions for memcpy(), ntohl() and htonl() */
-#include "../git-compat-util.h"
-
-#include "sha1.h"
-
-#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
-
-/*
- * Force usage of rol or ror by selecting the one with the smaller constant.
- * It _can_ generate slightly smaller code (a constant of 1 is special), but
- * perhaps more importantly it's possibly faster on any uarch that does a
- * rotate with a loop.
- */
-
-#define SHA_ASM(op, x, n) ({ unsigned int __res; __asm__(op " %1,%0":"=r" (__res):"i" (n), "0" (x)); __res; })
-#define SHA_ROL(x,n)	SHA_ASM("rol", x, n)
-#define SHA_ROR(x,n)	SHA_ASM("ror", x, n)
-
-#else
-
-#define SHA_ROT(X,l,r)	(((X) << (l)) | ((X) >> (r)))
-#define SHA_ROL(X,n)	SHA_ROT(X,n,32-(n))
-#define SHA_ROR(X,n)	SHA_ROT(X,32-(n),n)
-
-#endif
-
-/*
- * If you have 32 registers or more, the compiler can (and should)
- * try to change the array[] accesses into registers. However, on
- * machines with less than ~25 registers, that won't really work,
- * and at least gcc will make an unholy mess of it.
- *
- * So to avoid that mess which just slows things down, we force
- * the stores to memory to actually happen (we might be better off
- * with a 'W(t)=(val);asm("":"+m" (W(t))' there instead, as
- * suggested by Artur Skawina - that will also make gcc unable to
- * try to do the silly "optimize away loads" part because it won't
- * see what the value will be).
- *
- * Ben Herrenschmidt reports that on PPC, the C version comes close
- * to the optimized asm with this (ie on PPC you don't want that
- * 'volatile', since there are lots of registers).
- *
- * On ARM we get the best code generation by forcing a full memory barrier
- * between each SHA_ROUND, otherwise gcc happily get wild with spilling and
- * the stack frame size simply explode and performance goes down the drain.
- */
-
-#if defined(__i386__) || defined(__x86_64__)
-  #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))
-#elif defined(__GNUC__) && defined(__arm__)
-  #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
-#else
-  #define setW(x, val) (W(x) = (val))
-#endif
-
-/* This "rolls" over the 512-bit array */
-#define W(x) (array[(x)&15])
-
-/*
- * Where do we get the source from? The first 16 iterations get it from
- * the input data, the next mix it from the 512-bit array.
- */
-#define SHA_SRC(t) get_be32((unsigned char *) block + (t)*4)
-#define SHA_MIX(t) SHA_ROL(W((t)+13) ^ W((t)+8) ^ W((t)+2) ^ W(t), 1);
-
-#define SHA_ROUND(t, input, fn, constant, A, B, C, D, E) do { \
-	unsigned int TEMP = input(t); setW(t, TEMP); \
-	E += TEMP + SHA_ROL(A,5) + (fn) + (constant); \
-	B = SHA_ROR(B, 2); } while (0)
-
-#define T_0_15(t, A, B, C, D, E)  SHA_ROUND(t, SHA_SRC, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
-#define T_16_19(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
-#define T_20_39(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) , 0x6ed9eba1, A, B, C, D, E )
-#define T_40_59(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, ((B&C)+(D&(B^C))) , 0x8f1bbcdc, A, B, C, D, E )
-#define T_60_79(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) ,  0xca62c1d6, A, B, C, D, E )
-
-static void blk_SHA1_Block(blk_SHA_CTX *ctx, const void *block)
-{
-	unsigned int A,B,C,D,E;
-	unsigned int array[16];
-
-	A = ctx->H[0];
-	B = ctx->H[1];
-	C = ctx->H[2];
-	D = ctx->H[3];
-	E = ctx->H[4];
-
-	/* Round 1 - iterations 0-16 take their input from 'block' */
-	T_0_15( 0, A, B, C, D, E);
-	T_0_15( 1, E, A, B, C, D);
-	T_0_15( 2, D, E, A, B, C);
-	T_0_15( 3, C, D, E, A, B);
-	T_0_15( 4, B, C, D, E, A);
-	T_0_15( 5, A, B, C, D, E);
-	T_0_15( 6, E, A, B, C, D);
-	T_0_15( 7, D, E, A, B, C);
-	T_0_15( 8, C, D, E, A, B);
-	T_0_15( 9, B, C, D, E, A);
-	T_0_15(10, A, B, C, D, E);
-	T_0_15(11, E, A, B, C, D);
-	T_0_15(12, D, E, A, B, C);
-	T_0_15(13, C, D, E, A, B);
-	T_0_15(14, B, C, D, E, A);
-	T_0_15(15, A, B, C, D, E);
-
-	/* Round 1 - tail. Input from 512-bit mixing array */
-	T_16_19(16, E, A, B, C, D);
-	T_16_19(17, D, E, A, B, C);
-	T_16_19(18, C, D, E, A, B);
-	T_16_19(19, B, C, D, E, A);
-
-	/* Round 2 */
-	T_20_39(20, A, B, C, D, E);
-	T_20_39(21, E, A, B, C, D);
-	T_20_39(22, D, E, A, B, C);
-	T_20_39(23, C, D, E, A, B);
-	T_20_39(24, B, C, D, E, A);
-	T_20_39(25, A, B, C, D, E);
-	T_20_39(26, E, A, B, C, D);
-	T_20_39(27, D, E, A, B, C);
-	T_20_39(28, C, D, E, A, B);
-	T_20_39(29, B, C, D, E, A);
-	T_20_39(30, A, B, C, D, E);
-	T_20_39(31, E, A, B, C, D);
-	T_20_39(32, D, E, A, B, C);
-	T_20_39(33, C, D, E, A, B);
-	T_20_39(34, B, C, D, E, A);
-	T_20_39(35, A, B, C, D, E);
-	T_20_39(36, E, A, B, C, D);
-	T_20_39(37, D, E, A, B, C);
-	T_20_39(38, C, D, E, A, B);
-	T_20_39(39, B, C, D, E, A);
-
-	/* Round 3 */
-	T_40_59(40, A, B, C, D, E);
-	T_40_59(41, E, A, B, C, D);
-	T_40_59(42, D, E, A, B, C);
-	T_40_59(43, C, D, E, A, B);
-	T_40_59(44, B, C, D, E, A);
-	T_40_59(45, A, B, C, D, E);
-	T_40_59(46, E, A, B, C, D);
-	T_40_59(47, D, E, A, B, C);
-	T_40_59(48, C, D, E, A, B);
-	T_40_59(49, B, C, D, E, A);
-	T_40_59(50, A, B, C, D, E);
-	T_40_59(51, E, A, B, C, D);
-	T_40_59(52, D, E, A, B, C);
-	T_40_59(53, C, D, E, A, B);
-	T_40_59(54, B, C, D, E, A);
-	T_40_59(55, A, B, C, D, E);
-	T_40_59(56, E, A, B, C, D);
-	T_40_59(57, D, E, A, B, C);
-	T_40_59(58, C, D, E, A, B);
-	T_40_59(59, B, C, D, E, A);
-
-	/* Round 4 */
-	T_60_79(60, A, B, C, D, E);
-	T_60_79(61, E, A, B, C, D);
-	T_60_79(62, D, E, A, B, C);
-	T_60_79(63, C, D, E, A, B);
-	T_60_79(64, B, C, D, E, A);
-	T_60_79(65, A, B, C, D, E);
-	T_60_79(66, E, A, B, C, D);
-	T_60_79(67, D, E, A, B, C);
-	T_60_79(68, C, D, E, A, B);
-	T_60_79(69, B, C, D, E, A);
-	T_60_79(70, A, B, C, D, E);
-	T_60_79(71, E, A, B, C, D);
-	T_60_79(72, D, E, A, B, C);
-	T_60_79(73, C, D, E, A, B);
-	T_60_79(74, B, C, D, E, A);
-	T_60_79(75, A, B, C, D, E);
-	T_60_79(76, E, A, B, C, D);
-	T_60_79(77, D, E, A, B, C);
-	T_60_79(78, C, D, E, A, B);
-	T_60_79(79, B, C, D, E, A);
-
-	ctx->H[0] += A;
-	ctx->H[1] += B;
-	ctx->H[2] += C;
-	ctx->H[3] += D;
-	ctx->H[4] += E;
-}
-
-void blk_SHA1_Init(blk_SHA_CTX *ctx)
-{
-	ctx->size = 0;
-
-	/* Initialize H with the magic constants (see FIPS180 for constants) */
-	ctx->H[0] = 0x67452301;
-	ctx->H[1] = 0xefcdab89;
-	ctx->H[2] = 0x98badcfe;
-	ctx->H[3] = 0x10325476;
-	ctx->H[4] = 0xc3d2e1f0;
-}
-
-void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, unsigned long len)
-{
-	unsigned int lenW = ctx->size & 63;
-
-	ctx->size += len;
-
-	/* Read the data into W and process blocks as they get full */
-	if (lenW) {
-		unsigned int left = 64 - lenW;
-		if (len < left)
-			left = len;
-		memcpy(lenW + (char *)ctx->W, data, left);
-		lenW = (lenW + left) & 63;
-		len -= left;
-		data = ((const char *)data + left);
-		if (lenW)
-			return;
-		blk_SHA1_Block(ctx, ctx->W);
-	}
-	while (len >= 64) {
-		blk_SHA1_Block(ctx, data);
-		data = ((const char *)data + 64);
-		len -= 64;
-	}
-	if (len)
-		memcpy(ctx->W, data, len);
-}
-
-void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx)
-{
-	static const unsigned char pad[64] = { 0x80 };
-	unsigned int padlen[2];
-	int i;
-
-	/* Pad with a binary 1 (ie 0x80), then zeroes, then length */
-	padlen[0] = htonl((uint32_t)(ctx->size >> 29));
-	padlen[1] = htonl((uint32_t)(ctx->size << 3));
-
-	i = ctx->size & 63;
-	blk_SHA1_Update(ctx, pad, 1 + (63 & (55 - i)));
-	blk_SHA1_Update(ctx, padlen, 8);
-
-	/* Output hash */
-	for (i = 0; i < 5; i++)
-		put_be32(hashout + i * 4, ctx->H[i]);
-}
diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
deleted file mode 100644
index b864df6..0000000
--- a/block-sha1/sha1.h
+++ /dev/null
@@ -1,22 +0,0 @@
-/*
- * SHA1 routine optimized to do word accesses rather than byte accesses,
- * and to avoid unnecessary copies into the context array.
- *
- * This was initially based on the Mozilla SHA1 implementation, although
- * none of the original Mozilla code remains.
- */
-
-typedef struct {
-	unsigned long long size;
-	unsigned int H[5];
-	unsigned int W[16];
-} blk_SHA_CTX;
-
-void blk_SHA1_Init(blk_SHA_CTX *ctx);
-void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *dataIn, unsigned long len);
-void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx);
-
-#define git_SHA_CTX	blk_SHA_CTX
-#define git_SHA1_Init	blk_SHA1_Init
-#define git_SHA1_Update	blk_SHA1_Update
-#define git_SHA1_Final	blk_SHA1_Final
diff --git a/cache.h b/cache.h
index 182ac62..961da4e 100644
--- a/cache.h
+++ b/cache.h
@@ -28,7 +28,7 @@
 #ifndef git_SHA_CTX
 
 #ifdef SHA1_MAX_BLOCK_SIZE
-#include "compat/sha1-chunked.h"
+#include "sha1/chk-sha1.h"
 #define git_SHA_CTX		platform_SHA_CTX
 #define git_SHA1_Init		platform_SHA1_Init
 #define git_SHA1_Update		git_SHA1_Update_Chunked
diff --git a/compat/sha1-chunked.c b/compat/sha1-chunked.c
deleted file mode 100644
index 6adfcfd..0000000
--- a/compat/sha1-chunked.c
+++ /dev/null
@@ -1,19 +0,0 @@
-#include "cache.h"
-
-int git_SHA1_Update_Chunked(platform_SHA_CTX *c, const void *data, size_t len)
-{
-	size_t nr;
-	size_t total = 0;
-	const char *cdata = (const char*)data;
-
-	while (len) {
-		nr = len;
-		if (nr > SHA1_MAX_BLOCK_SIZE)
-			nr = SHA1_MAX_BLOCK_SIZE;
-		platform_SHA1_Update(c, cdata, nr);
-		total += nr;
-		cdata += nr;
-		len -= nr;
-	}
-	return total;
-}
diff --git a/compat/sha1-chunked.h b/compat/sha1-chunked.h
deleted file mode 100644
index 7b2df28..0000000
--- a/compat/sha1-chunked.h
+++ /dev/null
@@ -1,2 +0,0 @@
-
-int git_SHA1_Update_Chunked(platform_SHA_CTX *c, const void *data, size_t len);
diff --git a/ppc/sha1.c b/ppc/sha1.c
deleted file mode 100644
index ec6a192..0000000
--- a/ppc/sha1.c
+++ /dev/null
@@ -1,72 +0,0 @@
-/*
- * SHA-1 implementation.
- *
- * Copyright (C) 2005 Paul Mackerras <paulus@samba.org>
- *
- * This version assumes we are running on a big-endian machine.
- * It calls an external sha1_core() to process blocks of 64 bytes.
- */
-#include <stdio.h>
-#include <string.h>
-#include "sha1.h"
-
-extern void ppc_sha1_core(uint32_t *hash, const unsigned char *p,
-			  unsigned int nblocks);
-
-int ppc_SHA1_Init(ppc_SHA_CTX *c)
-{
-	c->hash[0] = 0x67452301;
-	c->hash[1] = 0xEFCDAB89;
-	c->hash[2] = 0x98BADCFE;
-	c->hash[3] = 0x10325476;
-	c->hash[4] = 0xC3D2E1F0;
-	c->len = 0;
-	c->cnt = 0;
-	return 0;
-}
-
-int ppc_SHA1_Update(ppc_SHA_CTX *c, const void *ptr, unsigned long n)
-{
-	unsigned long nb;
-	const unsigned char *p = ptr;
-
-	c->len += (uint64_t) n << 3;
-	while (n != 0) {
-		if (c->cnt || n < 64) {
-			nb = 64 - c->cnt;
-			if (nb > n)
-				nb = n;
-			memcpy(&c->buf.b[c->cnt], p, nb);
-			if ((c->cnt += nb) == 64) {
-				ppc_sha1_core(c->hash, c->buf.b, 1);
-				c->cnt = 0;
-			}
-		} else {
-			nb = n >> 6;
-			ppc_sha1_core(c->hash, p, nb);
-			nb <<= 6;
-		}
-		n -= nb;
-		p += nb;
-	}
-	return 0;
-}
-
-int ppc_SHA1_Final(unsigned char *hash, ppc_SHA_CTX *c)
-{
-	unsigned int cnt = c->cnt;
-
-	c->buf.b[cnt++] = 0x80;
-	if (cnt > 56) {
-		if (cnt < 64)
-			memset(&c->buf.b[cnt], 0, 64 - cnt);
-		ppc_sha1_core(c->hash, c->buf.b, 1);
-		cnt = 0;
-	}
-	if (cnt < 56)
-		memset(&c->buf.b[cnt], 0, 56 - cnt);
-	c->buf.l[7] = c->len;
-	ppc_sha1_core(c->hash, c->buf.b, 1);
-	memcpy(hash, c->hash, 20);
-	return 0;
-}
diff --git a/ppc/sha1.h b/ppc/sha1.h
deleted file mode 100644
index c405f73..0000000
--- a/ppc/sha1.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- * SHA-1 implementation.
- *
- * Copyright (C) 2005 Paul Mackerras <paulus@samba.org>
- */
-#include <stdint.h>
-
-typedef struct {
-	uint32_t hash[5];
-	uint32_t cnt;
-	uint64_t len;
-	union {
-		unsigned char b[64];
-		uint64_t l[8];
-	} buf;
-} ppc_SHA_CTX;
-
-int ppc_SHA1_Init(ppc_SHA_CTX *c);
-int ppc_SHA1_Update(ppc_SHA_CTX *c, const void *p, unsigned long n);
-int ppc_SHA1_Final(unsigned char *hash, ppc_SHA_CTX *c);
-
-#define git_SHA_CTX	ppc_SHA_CTX
-#define git_SHA1_Init	ppc_SHA1_Init
-#define git_SHA1_Update	ppc_SHA1_Update
-#define git_SHA1_Final	ppc_SHA1_Final
diff --git a/ppc/sha1ppc.S b/ppc/sha1ppc.S
deleted file mode 100644
index 1711eef..0000000
--- a/ppc/sha1ppc.S
+++ /dev/null
@@ -1,224 +0,0 @@
-/*
- * SHA-1 implementation for PowerPC.
- *
- * Copyright (C) 2005 Paul Mackerras <paulus@samba.org>
- */
-
-/*
- * PowerPC calling convention:
- * %r0 - volatile temp
- * %r1 - stack pointer.
- * %r2 - reserved
- * %r3-%r12 - Incoming arguments & return values; volatile.
- * %r13-%r31 - Callee-save registers
- * %lr - Return address, volatile
- * %ctr - volatile
- *
- * Register usage in this routine:
- * %r0 - temp
- * %r3 - argument (pointer to 5 words of SHA state)
- * %r4 - argument (pointer to data to hash)
- * %r5 - Constant K in SHA round (initially number of blocks to hash)
- * %r6-%r10 - Working copies of SHA variables A..E (actually E..A order)
- * %r11-%r26 - Data being hashed W[].
- * %r27-%r31 - Previous copies of A..E, for final add back.
- * %ctr - loop count
- */
-
-
-/*
- * We roll the registers for A, B, C, D, E around on each
- * iteration; E on iteration t is D on iteration t+1, and so on.
- * We use registers 6 - 10 for this.  (Registers 27 - 31 hold
- * the previous values.)
- */
-#define RA(t)	(((t)+4)%5+6)
-#define RB(t)	(((t)+3)%5+6)
-#define RC(t)	(((t)+2)%5+6)
-#define RD(t)	(((t)+1)%5+6)
-#define RE(t)	(((t)+0)%5+6)
-
-/* We use registers 11 - 26 for the W values */
-#define W(t)	((t)%16+11)
-
-/* Register 5 is used for the constant k */
-
-/*
- * The basic SHA-1 round function is:
- * E += ROTL(A,5) + F(B,C,D) + W[i] + K;  B = ROTL(B,30)
- * Then the variables are renamed: (A,B,C,D,E) = (E,A,B,C,D).
- *
- * Every 20 rounds, the function F() and the constant K changes:
- * - 20 rounds of f0(b,c,d) = "bit wise b ? c : d" =  (^b & d) + (b & c)
- * - 20 rounds of f1(b,c,d) = b^c^d = (b^d)^c
- * - 20 rounds of f2(b,c,d) = majority(b,c,d) = (b&d) + ((b^d)&c)
- * - 20 more rounds of f1(b,c,d)
- *
- * These are all scheduled for near-optimal performance on a G4.
- * The G4 is a 3-issue out-of-order machine with 3 ALUs, but it can only
- * *consider* starting the oldest 3 instructions per cycle.  So to get
- * maximum performance out of it, you have to treat it as an in-order
- * machine.  Which means interleaving the computation round t with the
- * computation of W[t+4].
- *
- * The first 16 rounds use W values loaded directly from memory, while the
- * remaining 64 use values computed from those first 16.  We preload
- * 4 values before starting, so there are three kinds of rounds:
- * - The first 12 (all f0) also load the W values from memory.
- * - The next 64 compute W(i+4) in parallel. 8*f0, 20*f1, 20*f2, 16*f1.
- * - The last 4 (all f1) do not do anything with W.
- *
- * Therefore, we have 6 different round functions:
- * STEPD0_LOAD(t,s) - Perform round t and load W(s).  s < 16
- * STEPD0_UPDATE(t,s) - Perform round t and compute W(s).  s >= 16.
- * STEPD1_UPDATE(t,s)
- * STEPD2_UPDATE(t,s)
- * STEPD1(t) - Perform round t with no load or update.
- *
- * The G5 is more fully out-of-order, and can find the parallelism
- * by itself.  The big limit is that it has a 2-cycle ALU latency, so
- * even though it's 2-way, the code has to be scheduled as if it's
- * 4-way, which can be a limit.  To help it, we try to schedule the
- * read of RA(t) as late as possible so it doesn't stall waiting for
- * the previous round's RE(t-1), and we try to rotate RB(t) as early
- * as possible while reading RC(t) (= RB(t-1)) as late as possible.
- */
-
-/* the initial loads. */
-#define LOADW(s) \
-	lwz	W(s),(s)*4(%r4)
-
-/*
- * Perform a step with F0, and load W(s).  Uses W(s) as a temporary
- * before loading it.
- * This is actually 10 instructions, which is an awkward fit.
- * It can execute grouped as listed, or delayed one instruction.
- * (If delayed two instructions, there is a stall before the start of the
- * second line.)  Thus, two iterations take 7 cycles, 3.5 cycles per round.
- */
-#define STEPD0_LOAD(t,s) \
-add RE(t),RE(t),W(t); andc   %r0,RD(t),RB(t);  and    W(s),RC(t),RB(t); \
-add RE(t),RE(t),%r0;  rotlwi %r0,RA(t),5;      rotlwi RB(t),RB(t),30;   \
-add RE(t),RE(t),W(s); add    %r0,%r0,%r5;      lwz    W(s),(s)*4(%r4);  \
-add RE(t),RE(t),%r0
-
-/*
- * This is likewise awkward, 13 instructions.  However, it can also
- * execute starting with 2 out of 3 possible moduli, so it does 2 rounds
- * in 9 cycles, 4.5 cycles/round.
- */
-#define STEPD0_UPDATE(t,s,loadk...) \
-add RE(t),RE(t),W(t); andc   %r0,RD(t),RB(t); xor    W(s),W((s)-16),W((s)-3); \
-add RE(t),RE(t),%r0;  and    %r0,RC(t),RB(t); xor    W(s),W(s),W((s)-8);      \
-add RE(t),RE(t),%r0;  rotlwi %r0,RA(t),5;     xor    W(s),W(s),W((s)-14);     \
-add RE(t),RE(t),%r5;  loadk; rotlwi RB(t),RB(t),30;  rotlwi W(s),W(s),1;     \
-add RE(t),RE(t),%r0
-
-/* Nicely optimal.  Conveniently, also the most common. */
-#define STEPD1_UPDATE(t,s,loadk...) \
-add RE(t),RE(t),W(t); xor    %r0,RD(t),RB(t); xor    W(s),W((s)-16),W((s)-3); \
-add RE(t),RE(t),%r5;  loadk; xor %r0,%r0,RC(t);  xor W(s),W(s),W((s)-8);      \
-add RE(t),RE(t),%r0;  rotlwi %r0,RA(t),5;     xor    W(s),W(s),W((s)-14);     \
-add RE(t),RE(t),%r0;  rotlwi RB(t),RB(t),30;  rotlwi W(s),W(s),1
-
-/*
- * The naked version, no UPDATE, for the last 4 rounds.  3 cycles per.
- * We could use W(s) as a temp register, but we don't need it.
- */
-#define STEPD1(t) \
-                        add   RE(t),RE(t),W(t); xor    %r0,RD(t),RB(t); \
-rotlwi RB(t),RB(t),30;  add   RE(t),RE(t),%r5;  xor    %r0,%r0,RC(t);   \
-add    RE(t),RE(t),%r0; rotlwi %r0,RA(t),5;     /* spare slot */        \
-add    RE(t),RE(t),%r0
-
-/*
- * 14 instructions, 5 cycles per.  The majority function is a bit
- * awkward to compute.  This can execute with a 1-instruction delay,
- * but it causes a 2-instruction delay, which triggers a stall.
- */
-#define STEPD2_UPDATE(t,s,loadk...) \
-add RE(t),RE(t),W(t); and    %r0,RD(t),RB(t); xor    W(s),W((s)-16),W((s)-3); \
-add RE(t),RE(t),%r0;  xor    %r0,RD(t),RB(t); xor    W(s),W(s),W((s)-8);      \
-add RE(t),RE(t),%r5;  loadk; and %r0,%r0,RC(t);  xor W(s),W(s),W((s)-14);     \
-add RE(t),RE(t),%r0;  rotlwi %r0,RA(t),5;     rotlwi W(s),W(s),1;             \
-add RE(t),RE(t),%r0;  rotlwi RB(t),RB(t),30
-
-#define STEP0_LOAD4(t,s)		\
-	STEPD0_LOAD(t,s);		\
-	STEPD0_LOAD((t+1),(s)+1);	\
-	STEPD0_LOAD((t)+2,(s)+2);	\
-	STEPD0_LOAD((t)+3,(s)+3)
-
-#define STEPUP4(fn, t, s, loadk...)		\
-	STEP##fn##_UPDATE(t,s,);		\
-	STEP##fn##_UPDATE((t)+1,(s)+1,);	\
-	STEP##fn##_UPDATE((t)+2,(s)+2,);	\
-	STEP##fn##_UPDATE((t)+3,(s)+3,loadk)
-
-#define STEPUP20(fn, t, s, loadk...)	\
-	STEPUP4(fn, t, s,);		\
-	STEPUP4(fn, (t)+4, (s)+4,);	\
-	STEPUP4(fn, (t)+8, (s)+8,);	\
-	STEPUP4(fn, (t)+12, (s)+12,);	\
-	STEPUP4(fn, (t)+16, (s)+16, loadk)
-
-	.globl	ppc_sha1_core
-ppc_sha1_core:
-	stwu	%r1,-80(%r1)
-	stmw	%r13,4(%r1)
-
-	/* Load up A - E */
-	lmw	%r27,0(%r3)
-
-	mtctr	%r5
-
-1:
-	LOADW(0)
-	lis	%r5,0x5a82
-	mr	RE(0),%r31
-	LOADW(1)
-	mr	RD(0),%r30
-	mr	RC(0),%r29
-	LOADW(2)
-	ori	%r5,%r5,0x7999	/* K0-19 */
-	mr	RB(0),%r28
-	LOADW(3)
-	mr	RA(0),%r27
-
-	STEP0_LOAD4(0, 4)
-	STEP0_LOAD4(4, 8)
-	STEP0_LOAD4(8, 12)
-	STEPUP4(D0, 12, 16,)
-	STEPUP4(D0, 16, 20, lis %r5,0x6ed9)
-
-	ori	%r5,%r5,0xeba1	/* K20-39 */
-	STEPUP20(D1, 20, 24, lis %r5,0x8f1b)
-
-	ori	%r5,%r5,0xbcdc	/* K40-59 */
-	STEPUP20(D2, 40, 44, lis %r5,0xca62)
-
-	ori	%r5,%r5,0xc1d6	/* K60-79 */
-	STEPUP4(D1, 60, 64,)
-	STEPUP4(D1, 64, 68,)
-	STEPUP4(D1, 68, 72,)
-	STEPUP4(D1, 72, 76,)
-	addi	%r4,%r4,64
-	STEPD1(76)
-	STEPD1(77)
-	STEPD1(78)
-	STEPD1(79)
-
-	/* Add results to original values */
-	add	%r31,%r31,RE(0)
-	add	%r30,%r30,RD(0)
-	add	%r29,%r29,RC(0)
-	add	%r28,%r28,RB(0)
-	add	%r27,%r27,RA(0)
-
-	bdnz	1b
-
-	/* Save final hash, restore registers, and return */
-	stmw	%r27,0(%r3)
-	lmw	%r13,4(%r1)
-	addi	%r1,%r1,80
-	blr
diff --git a/sha1/blk-sha1.c b/sha1/blk-sha1.c
new file mode 100644
index 0000000..abacf05
--- /dev/null
+++ b/sha1/blk-sha1.c
@@ -0,0 +1,251 @@
+/*
+ * SHA1 routine optimized to do word accesses rather than byte accesses,
+ * and to avoid unnecessary copies into the context array.
+ *
+ * This was initially based on the Mozilla SHA1 implementation, although
+ * none of the original Mozilla code remains.
+ */
+
+/* this is only to get definitions for memcpy(), ntohl() and htonl() */
+#include "../git-compat-util.h"
+
+#include "blk-sha1.h"
+
+#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
+
+/*
+ * Force usage of rol or ror by selecting the one with the smaller constant.
+ * It _can_ generate slightly smaller code (a constant of 1 is special), but
+ * perhaps more importantly it's possibly faster on any uarch that does a
+ * rotate with a loop.
+ */
+
+#define SHA_ASM(op, x, n) ({ unsigned int __res; __asm__(op " %1,%0":"=r" (__res):"i" (n), "0" (x)); __res; })
+#define SHA_ROL(x,n)	SHA_ASM("rol", x, n)
+#define SHA_ROR(x,n)	SHA_ASM("ror", x, n)
+
+#else
+
+#define SHA_ROT(X,l,r)	(((X) << (l)) | ((X) >> (r)))
+#define SHA_ROL(X,n)	SHA_ROT(X,n,32-(n))
+#define SHA_ROR(X,n)	SHA_ROT(X,32-(n),n)
+
+#endif
+
+/*
+ * If you have 32 registers or more, the compiler can (and should)
+ * try to change the array[] accesses into registers. However, on
+ * machines with less than ~25 registers, that won't really work,
+ * and at least gcc will make an unholy mess of it.
+ *
+ * So to avoid that mess which just slows things down, we force
+ * the stores to memory to actually happen (we might be better off
+ * with a 'W(t)=(val);asm("":"+m" (W(t))' there instead, as
+ * suggested by Artur Skawina - that will also make gcc unable to
+ * try to do the silly "optimize away loads" part because it won't
+ * see what the value will be).
+ *
+ * Ben Herrenschmidt reports that on PPC, the C version comes close
+ * to the optimized asm with this (ie on PPC you don't want that
+ * 'volatile', since there are lots of registers).
+ *
+ * On ARM we get the best code generation by forcing a full memory barrier
+ * between each SHA_ROUND, otherwise gcc happily get wild with spilling and
+ * the stack frame size simply explode and performance goes down the drain.
+ */
+
+#if defined(__i386__) || defined(__x86_64__)
+  #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))
+#elif defined(__GNUC__) && defined(__arm__)
+  #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
+#else
+  #define setW(x, val) (W(x) = (val))
+#endif
+
+/* This "rolls" over the 512-bit array */
+#define W(x) (array[(x)&15])
+
+/*
+ * Where do we get the source from? The first 16 iterations get it from
+ * the input data, the next mix it from the 512-bit array.
+ */
+#define SHA_SRC(t) get_be32((unsigned char *) block + (t)*4)
+#define SHA_MIX(t) SHA_ROL(W((t)+13) ^ W((t)+8) ^ W((t)+2) ^ W(t), 1);
+
+#define SHA_ROUND(t, input, fn, constant, A, B, C, D, E) do { \
+	unsigned int TEMP = input(t); setW(t, TEMP); \
+	E += TEMP + SHA_ROL(A,5) + (fn) + (constant); \
+	B = SHA_ROR(B, 2); } while (0)
+
+#define T_0_15(t, A, B, C, D, E)  SHA_ROUND(t, SHA_SRC, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
+#define T_16_19(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (((C^D)&B)^D) , 0x5a827999, A, B, C, D, E )
+#define T_20_39(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) , 0x6ed9eba1, A, B, C, D, E )
+#define T_40_59(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, ((B&C)+(D&(B^C))) , 0x8f1bbcdc, A, B, C, D, E )
+#define T_60_79(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) ,  0xca62c1d6, A, B, C, D, E )
+
+static void blk_SHA1_Block(blk_SHA_CTX *ctx, const void *block)
+{
+	unsigned int A,B,C,D,E;
+	unsigned int array[16];
+
+	A = ctx->H[0];
+	B = ctx->H[1];
+	C = ctx->H[2];
+	D = ctx->H[3];
+	E = ctx->H[4];
+
+	/* Round 1 - iterations 0-16 take their input from 'block' */
+	T_0_15( 0, A, B, C, D, E);
+	T_0_15( 1, E, A, B, C, D);
+	T_0_15( 2, D, E, A, B, C);
+	T_0_15( 3, C, D, E, A, B);
+	T_0_15( 4, B, C, D, E, A);
+	T_0_15( 5, A, B, C, D, E);
+	T_0_15( 6, E, A, B, C, D);
+	T_0_15( 7, D, E, A, B, C);
+	T_0_15( 8, C, D, E, A, B);
+	T_0_15( 9, B, C, D, E, A);
+	T_0_15(10, A, B, C, D, E);
+	T_0_15(11, E, A, B, C, D);
+	T_0_15(12, D, E, A, B, C);
+	T_0_15(13, C, D, E, A, B);
+	T_0_15(14, B, C, D, E, A);
+	T_0_15(15, A, B, C, D, E);
+
+	/* Round 1 - tail. Input from 512-bit mixing array */
+	T_16_19(16, E, A, B, C, D);
+	T_16_19(17, D, E, A, B, C);
+	T_16_19(18, C, D, E, A, B);
+	T_16_19(19, B, C, D, E, A);
+
+	/* Round 2 */
+	T_20_39(20, A, B, C, D, E);
+	T_20_39(21, E, A, B, C, D);
+	T_20_39(22, D, E, A, B, C);
+	T_20_39(23, C, D, E, A, B);
+	T_20_39(24, B, C, D, E, A);
+	T_20_39(25, A, B, C, D, E);
+	T_20_39(26, E, A, B, C, D);
+	T_20_39(27, D, E, A, B, C);
+	T_20_39(28, C, D, E, A, B);
+	T_20_39(29, B, C, D, E, A);
+	T_20_39(30, A, B, C, D, E);
+	T_20_39(31, E, A, B, C, D);
+	T_20_39(32, D, E, A, B, C);
+	T_20_39(33, C, D, E, A, B);
+	T_20_39(34, B, C, D, E, A);
+	T_20_39(35, A, B, C, D, E);
+	T_20_39(36, E, A, B, C, D);
+	T_20_39(37, D, E, A, B, C);
+	T_20_39(38, C, D, E, A, B);
+	T_20_39(39, B, C, D, E, A);
+
+	/* Round 3 */
+	T_40_59(40, A, B, C, D, E);
+	T_40_59(41, E, A, B, C, D);
+	T_40_59(42, D, E, A, B, C);
+	T_40_59(43, C, D, E, A, B);
+	T_40_59(44, B, C, D, E, A);
+	T_40_59(45, A, B, C, D, E);
+	T_40_59(46, E, A, B, C, D);
+	T_40_59(47, D, E, A, B, C);
+	T_40_59(48, C, D, E, A, B);
+	T_40_59(49, B, C, D, E, A);
+	T_40_59(50, A, B, C, D, E);
+	T_40_59(51, E, A, B, C, D);
+	T_40_59(52, D, E, A, B, C);
+	T_40_59(53, C, D, E, A, B);
+	T_40_59(54, B, C, D, E, A);
+	T_40_59(55, A, B, C, D, E);
+	T_40_59(56, E, A, B, C, D);
+	T_40_59(57, D, E, A, B, C);
+	T_40_59(58, C, D, E, A, B);
+	T_40_59(59, B, C, D, E, A);
+
+	/* Round 4 */
+	T_60_79(60, A, B, C, D, E);
+	T_60_79(61, E, A, B, C, D);
+	T_60_79(62, D, E, A, B, C);
+	T_60_79(63, C, D, E, A, B);
+	T_60_79(64, B, C, D, E, A);
+	T_60_79(65, A, B, C, D, E);
+	T_60_79(66, E, A, B, C, D);
+	T_60_79(67, D, E, A, B, C);
+	T_60_79(68, C, D, E, A, B);
+	T_60_79(69, B, C, D, E, A);
+	T_60_79(70, A, B, C, D, E);
+	T_60_79(71, E, A, B, C, D);
+	T_60_79(72, D, E, A, B, C);
+	T_60_79(73, C, D, E, A, B);
+	T_60_79(74, B, C, D, E, A);
+	T_60_79(75, A, B, C, D, E);
+	T_60_79(76, E, A, B, C, D);
+	T_60_79(77, D, E, A, B, C);
+	T_60_79(78, C, D, E, A, B);
+	T_60_79(79, B, C, D, E, A);
+
+	ctx->H[0] += A;
+	ctx->H[1] += B;
+	ctx->H[2] += C;
+	ctx->H[3] += D;
+	ctx->H[4] += E;
+}
+
+void blk_SHA1_Init(blk_SHA_CTX *ctx)
+{
+	ctx->size = 0;
+
+	/* Initialize H with the magic constants (see FIPS180 for constants) */
+	ctx->H[0] = 0x67452301;
+	ctx->H[1] = 0xefcdab89;
+	ctx->H[2] = 0x98badcfe;
+	ctx->H[3] = 0x10325476;
+	ctx->H[4] = 0xc3d2e1f0;
+}
+
+void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, unsigned long len)
+{
+	unsigned int lenW = ctx->size & 63;
+
+	ctx->size += len;
+
+	/* Read the data into W and process blocks as they get full */
+	if (lenW) {
+		unsigned int left = 64 - lenW;
+		if (len < left)
+			left = len;
+		memcpy(lenW + (char *)ctx->W, data, left);
+		lenW = (lenW + left) & 63;
+		len -= left;
+		data = ((const char *)data + left);
+		if (lenW)
+			return;
+		blk_SHA1_Block(ctx, ctx->W);
+	}
+	while (len >= 64) {
+		blk_SHA1_Block(ctx, data);
+		data = ((const char *)data + 64);
+		len -= 64;
+	}
+	if (len)
+		memcpy(ctx->W, data, len);
+}
+
+void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx)
+{
+	static const unsigned char pad[64] = { 0x80 };
+	unsigned int padlen[2];
+	int i;
+
+	/* Pad with a binary 1 (ie 0x80), then zeroes, then length */
+	padlen[0] = htonl((uint32_t)(ctx->size >> 29));
+	padlen[1] = htonl((uint32_t)(ctx->size << 3));
+
+	i = ctx->size & 63;
+	blk_SHA1_Update(ctx, pad, 1 + (63 & (55 - i)));
+	blk_SHA1_Update(ctx, padlen, 8);
+
+	/* Output hash */
+	for (i = 0; i < 5; i++)
+		put_be32(hashout + i * 4, ctx->H[i]);
+}
diff --git a/sha1/blk-sha1.h b/sha1/blk-sha1.h
new file mode 100644
index 0000000..b864df6
--- /dev/null
+++ b/sha1/blk-sha1.h
@@ -0,0 +1,22 @@
+/*
+ * SHA1 routine optimized to do word accesses rather than byte accesses,
+ * and to avoid unnecessary copies into the context array.
+ *
+ * This was initially based on the Mozilla SHA1 implementation, although
+ * none of the original Mozilla code remains.
+ */
+
+typedef struct {
+	unsigned long long size;
+	unsigned int H[5];
+	unsigned int W[16];
+} blk_SHA_CTX;
+
+void blk_SHA1_Init(blk_SHA_CTX *ctx);
+void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *dataIn, unsigned long len);
+void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx);
+
+#define git_SHA_CTX	blk_SHA_CTX
+#define git_SHA1_Init	blk_SHA1_Init
+#define git_SHA1_Update	blk_SHA1_Update
+#define git_SHA1_Final	blk_SHA1_Final
diff --git a/sha1/chk-sha1.c b/sha1/chk-sha1.c
new file mode 100644
index 0000000..6adfcfd
--- /dev/null
+++ b/sha1/chk-sha1.c
@@ -0,0 +1,19 @@
+#include "cache.h"
+
+int git_SHA1_Update_Chunked(platform_SHA_CTX *c, const void *data, size_t len)
+{
+	size_t nr;
+	size_t total = 0;
+	const char *cdata = (const char*)data;
+
+	while (len) {
+		nr = len;
+		if (nr > SHA1_MAX_BLOCK_SIZE)
+			nr = SHA1_MAX_BLOCK_SIZE;
+		platform_SHA1_Update(c, cdata, nr);
+		total += nr;
+		cdata += nr;
+		len -= nr;
+	}
+	return total;
+}
diff --git a/sha1/chk-sha1.h b/sha1/chk-sha1.h
new file mode 100644
index 0000000..7b2df28
--- /dev/null
+++ b/sha1/chk-sha1.h
@@ -0,0 +1,2 @@
+
+int git_SHA1_Update_Chunked(platform_SHA_CTX *c, const void *data, size_t len);
diff --git a/sha1/ppc-sha1.c b/sha1/ppc-sha1.c
new file mode 100644
index 0000000..2ca9a5a
--- /dev/null
+++ b/sha1/ppc-sha1.c
@@ -0,0 +1,72 @@
+/*
+ * SHA-1 implementation.
+ *
+ * Copyright (C) 2005 Paul Mackerras <paulus@samba.org>
+ *
+ * This version assumes we are running on a big-endian machine.
+ * It calls an external sha1_core() to process blocks of 64 bytes.
+ */
+#include <stdio.h>
+#include <string.h>
+#include "ppc-sha1.h"
+
+extern void ppc_sha1_core(uint32_t *hash, const unsigned char *p,
+			  unsigned int nblocks);
+
+int ppc_SHA1_Init(ppc_SHA_CTX *c)
+{
+	c->hash[0] = 0x67452301;
+	c->hash[1] = 0xEFCDAB89;
+	c->hash[2] = 0x98BADCFE;
+	c->hash[3] = 0x10325476;
+	c->hash[4] = 0xC3D2E1F0;
+	c->len = 0;
+	c->cnt = 0;
+	return 0;
+}
+
+int ppc_SHA1_Update(ppc_SHA_CTX *c, const void *ptr, unsigned long n)
+{
+	unsigned long nb;
+	const unsigned char *p = ptr;
+
+	c->len += (uint64_t) n << 3;
+	while (n != 0) {
+		if (c->cnt || n < 64) {
+			nb = 64 - c->cnt;
+			if (nb > n)
+				nb = n;
+			memcpy(&c->buf.b[c->cnt], p, nb);
+			if ((c->cnt += nb) == 64) {
+				ppc_sha1_core(c->hash, c->buf.b, 1);
+				c->cnt = 0;
+			}
+		} else {
+			nb = n >> 6;
+			ppc_sha1_core(c->hash, p, nb);
+			nb <<= 6;
+		}
+		n -= nb;
+		p += nb;
+	}
+	return 0;
+}
+
+int ppc_SHA1_Final(unsigned char *hash, ppc_SHA_CTX *c)
+{
+	unsigned int cnt = c->cnt;
+
+	c->buf.b[cnt++] = 0x80;
+	if (cnt > 56) {
+		if (cnt < 64)
+			memset(&c->buf.b[cnt], 0, 64 - cnt);
+		ppc_sha1_core(c->hash, c->buf.b, 1);
+		cnt = 0;
+	}
+	if (cnt < 56)
+		memset(&c->buf.b[cnt], 0, 56 - cnt);
+	c->buf.l[7] = c->len;
+	ppc_sha1_core(c->hash, c->buf.b, 1);
+	memcpy(hash, c->hash, 20);
+	return 0;
+}
diff --git a/sha1/ppc-sha1.h b/sha1/ppc-sha1.h
new file mode 100644
index 0000000..c405f73
--- /dev/null
+++ b/sha1/ppc-sha1.h
@@ -0,0 +1,25 @@
+/*
+ * SHA-1 implementation.
+ *
+ * Copyright (C) 2005 Paul Mackerras <paulus@samba.org>
+ */
+#include <stdint.h>
+
+typedef struct {
+	uint32_t hash[5];
+	uint32_t cnt;
+	uint64_t len;
+	union {
+		unsigned char b[64];
+		uint64_t l[8];
+	} buf;
+} ppc_SHA_CTX;
+
+int ppc_SHA1_Init(ppc_SHA_CTX *c);
+int ppc_SHA1_Update(ppc_SHA_CTX *c, const void *p, unsigned long n);
+int ppc_SHA1_Final(unsigned char *hash, ppc_SHA_CTX *c);
+
+#define git_SHA_CTX	ppc_SHA_CTX
+#define git_SHA1_Init	ppc_SHA1_Init
+#define git_SHA1_Update	ppc_SHA1_Update
+#define git_SHA1_Final	ppc_SHA1_Final
diff --git a/sha1/ppc-sha1asm.S b/sha1/ppc-sha1asm.S
new file mode 100644
index 0000000..1711eef
--- /dev/null
+++ b/sha1/ppc-sha1asm.S
@@ -0,0 +1,224 @@
+/*
+ * SHA-1 implementation for PowerPC.
+ *
+ * Copyright (C) 2005 Paul Mackerras <paulus@samba.org>
+ */
+
+/*
+ * PowerPC calling convention:
+ * %r0 - volatile temp
+ * %r1 - stack pointer.
+ * %r2 - reserved
+ * %r3-%r12 - Incoming arguments & return values; volatile.
+ * %r13-%r31 - Callee-save registers
+ * %lr - Return address, volatile
+ * %ctr - volatile
+ *
+ * Register usage in this routine:
+ * %r0 - temp
+ * %r3 - argument (pointer to 5 words of SHA state)
+ * %r4 - argument (pointer to data to hash)
+ * %r5 - Constant K in SHA round (initially number of blocks to hash)
+ * %r6-%r10 - Working copies of SHA variables A..E (actually E..A order)
+ * %r11-%r26 - Data being hashed W[].
+ * %r27-%r31 - Previous copies of A..E, for final add back.
+ * %ctr - loop count
+ */
+
+
+/*
+ * We roll the registers for A, B, C, D, E around on each
+ * iteration; E on iteration t is D on iteration t+1, and so on.
+ * We use registers 6 - 10 for this.  (Registers 27 - 31 hold
+ * the previous values.)
+ */
+#define RA(t)	(((t)+4)%5+6)
+#define RB(t)	(((t)+3)%5+6)
+#define RC(t)	(((t)+2)%5+6)
+#define RD(t)	(((t)+1)%5+6)
+#define RE(t)	(((t)+0)%5+6)
+
+/* We use registers 11 - 26 for the W values */
+#define W(t)	((t)%16+11)
+
+/* Register 5 is used for the constant k */
+
+/*
+ * The basic SHA-1 round function is:
+ * E += ROTL(A,5) + F(B,C,D) + W[i] + K;  B = ROTL(B,30)
+ * Then the variables are renamed: (A,B,C,D,E) = (E,A,B,C,D).
+ *
+ * Every 20 rounds, the function F() and the constant K changes:
+ * - 20 rounds of f0(b,c,d) = "bit wise b ? c : d" =  (^b & d) + (b & c)
+ * - 20 rounds of f1(b,c,d) = b^c^d = (b^d)^c
+ * - 20 rounds of f2(b,c,d) = majority(b,c,d) = (b&d) + ((b^d)&c)
+ * - 20 more rounds of f1(b,c,d)
+ *
+ * These are all scheduled for near-optimal performance on a G4.
+ * The G4 is a 3-issue out-of-order machine with 3 ALUs, but it can only
+ * *consider* starting the oldest 3 instructions per cycle.  So to get
+ * maximum performance out of it, you have to treat it as an in-order
+ * machine.  Which means interleaving the computation round t with the
+ * computation of W[t+4].
+ *
+ * The first 16 rounds use W values loaded directly from memory, while the
+ * remaining 64 use values computed from those first 16.  We preload
+ * 4 values before starting, so there are three kinds of rounds:
+ * - The first 12 (all f0) also load the W values from memory.
+ * - The next 64 compute W(i+4) in parallel. 8*f0, 20*f1, 20*f2, 16*f1.
+ * - The last 4 (all f1) do not do anything with W.
+ *
+ * Therefore, we have 6 different round functions:
+ * STEPD0_LOAD(t,s) - Perform round t and load W(s).  s < 16
+ * STEPD0_UPDATE(t,s) - Perform round t and compute W(s).  s >= 16.
+ * STEPD1_UPDATE(t,s)
+ * STEPD2_UPDATE(t,s)
+ * STEPD1(t) - Perform round t with no load or update.
+ *
+ * The G5 is more fully out-of-order, and can find the parallelism
+ * by itself.  The big limit is that it has a 2-cycle ALU latency, so
+ * even though it's 2-way, the code has to be scheduled as if it's
+ * 4-way, which can be a limit.  To help it, we try to schedule the
+ * read of RA(t) as late as possible so it doesn't stall waiting for
+ * the previous round's RE(t-1), and we try to rotate RB(t) as early
+ * as possible while reading RC(t) (= RB(t-1)) as late as possible.
+ */
+
+/* the initial loads. */
+#define LOADW(s) \
+	lwz	W(s),(s)*4(%r4)
+
+/*
+ * Perform a step with F0, and load W(s).  Uses W(s) as a temporary
+ * before loading it.
+ * This is actually 10 instructions, which is an awkward fit.
+ * It can execute grouped as listed, or delayed one instruction.
+ * (If delayed two instructions, there is a stall before the start of the
+ * second line.)  Thus, two iterations take 7 cycles, 3.5 cycles per round.
+ */
+#define STEPD0_LOAD(t,s) \
+add RE(t),RE(t),W(t); andc   %r0,RD(t),RB(t);  and    W(s),RC(t),RB(t); \
+add RE(t),RE(t),%r0;  rotlwi %r0,RA(t),5;      rotlwi RB(t),RB(t),30;   \
+add RE(t),RE(t),W(s); add    %r0,%r0,%r5;      lwz    W(s),(s)*4(%r4);  \
+add RE(t),RE(t),%r0
+
+/*
+ * This is likewise awkward, 13 instructions.  However, it can also
+ * execute starting with 2 out of 3 possible moduli, so it does 2 rounds
+ * in 9 cycles, 4.5 cycles/round.
+ */
+#define STEPD0_UPDATE(t,s,loadk...) \
+add RE(t),RE(t),W(t); andc   %r0,RD(t),RB(t); xor    W(s),W((s)-16),W((s)-3); \
+add RE(t),RE(t),%r0;  and    %r0,RC(t),RB(t); xor    W(s),W(s),W((s)-8);      \
+add RE(t),RE(t),%r0;  rotlwi %r0,RA(t),5;     xor    W(s),W(s),W((s)-14);     \
+add RE(t),RE(t),%r5;  loadk; rotlwi RB(t),RB(t),30;  rotlwi W(s),W(s),1;     \
+add RE(t),RE(t),%r0
+
+/* Nicely optimal.  Conveniently, also the most common. */
+#define STEPD1_UPDATE(t,s,loadk...) \
+add RE(t),RE(t),W(t); xor    %r0,RD(t),RB(t); xor    W(s),W((s)-16),W((s)-3); \
+add RE(t),RE(t),%r5;  loadk; xor %r0,%r0,RC(t);  xor W(s),W(s),W((s)-8);      \
+add RE(t),RE(t),%r0;  rotlwi %r0,RA(t),5;     xor    W(s),W(s),W((s)-14);     \
+add RE(t),RE(t),%r0;  rotlwi RB(t),RB(t),30;  rotlwi W(s),W(s),1
+
+/*
+ * The naked version, no UPDATE, for the last 4 rounds.  3 cycles per.
+ * We could use W(s) as a temp register, but we don't need it.
+ */
+#define STEPD1(t) \
+                        add   RE(t),RE(t),W(t); xor    %r0,RD(t),RB(t); \
+rotlwi RB(t),RB(t),30;  add   RE(t),RE(t),%r5;  xor    %r0,%r0,RC(t);   \
+add    RE(t),RE(t),%r0; rotlwi %r0,RA(t),5;     /* spare slot */        \
+add    RE(t),RE(t),%r0
+
+/*
+ * 14 instructions, 5 cycles per.  The majority function is a bit
+ * awkward to compute.  This can execute with a 1-instruction delay,
+ * but it causes a 2-instruction delay, which triggers a stall.
+ */
+#define STEPD2_UPDATE(t,s,loadk...) \
+add RE(t),RE(t),W(t); and    %r0,RD(t),RB(t); xor    W(s),W((s)-16),W((s)-3); \
+add RE(t),RE(t),%r0;  xor    %r0,RD(t),RB(t); xor    W(s),W(s),W((s)-8);      \
+add RE(t),RE(t),%r5;  loadk; and %r0,%r0,RC(t);  xor W(s),W(s),W((s)-14);     \
+add RE(t),RE(t),%r0;  rotlwi %r0,RA(t),5;     rotlwi W(s),W(s),1;             \
+add RE(t),RE(t),%r0;  rotlwi RB(t),RB(t),30
+
+#define STEP0_LOAD4(t,s)		\
+	STEPD0_LOAD(t,s);		\
+	STEPD0_LOAD((t+1),(s)+1);	\
+	STEPD0_LOAD((t)+2,(s)+2);	\
+	STEPD0_LOAD((t)+3,(s)+3)
+
+#define STEPUP4(fn, t, s, loadk...)		\
+	STEP##fn##_UPDATE(t,s,);		\
+	STEP##fn##_UPDATE((t)+1,(s)+1,);	\
+	STEP##fn##_UPDATE((t)+2,(s)+2,);	\
+	STEP##fn##_UPDATE((t)+3,(s)+3,loadk)
+
+#define STEPUP20(fn, t, s, loadk...)	\
+	STEPUP4(fn, t, s,);		\
+	STEPUP4(fn, (t)+4, (s)+4,);	\
+	STEPUP4(fn, (t)+8, (s)+8,);	\
+	STEPUP4(fn, (t)+12, (s)+12,);	\
+	STEPUP4(fn, (t)+16, (s)+16, loadk)
+
+	.globl	ppc_sha1_core
+ppc_sha1_core:
+	stwu	%r1,-80(%r1)
+	stmw	%r13,4(%r1)
+
+	/* Load up A - E */
+	lmw	%r27,0(%r3)
+
+	mtctr	%r5
+
+1:
+	LOADW(0)
+	lis	%r5,0x5a82
+	mr	RE(0),%r31
+	LOADW(1)
+	mr	RD(0),%r30
+	mr	RC(0),%r29
+	LOADW(2)
+	ori	%r5,%r5,0x7999	/* K0-19 */
+	mr	RB(0),%r28
+	LOADW(3)
+	mr	RA(0),%r27
+
+	STEP0_LOAD4(0, 4)
+	STEP0_LOAD4(4, 8)
+	STEP0_LOAD4(8, 12)
+	STEPUP4(D0, 12, 16,)
+	STEPUP4(D0, 16, 20, lis %r5,0x6ed9)
+
+	ori	%r5,%r5,0xeba1	/* K20-39 */
+	STEPUP20(D1, 20, 24, lis %r5,0x8f1b)
+
+	ori	%r5,%r5,0xbcdc	/* K40-59 */
+	STEPUP20(D2, 40, 44, lis %r5,0xca62)
+
+	ori	%r5,%r5,0xc1d6	/* K60-79 */
+	STEPUP4(D1, 60, 64,)
+	STEPUP4(D1, 64, 68,)
+	STEPUP4(D1, 68, 72,)
+	STEPUP4(D1, 72, 76,)
+	addi	%r4,%r4,64
+	STEPD1(76)
+	STEPD1(77)
+	STEPD1(78)
+	STEPD1(79)
+
+	/* Add results to original values */
+	add	%r31,%r31,RE(0)
+	add	%r30,%r30,RD(0)
+	add	%r29,%r29,RC(0)
+	add	%r28,%r28,RB(0)
+	add	%r27,%r27,RA(0)
+
+	bdnz	1b
+
+	/* Save final hash, restore registers, and return */
+	stmw	%r27,0(%r3)
+	lmw	%r13,4(%r1)
+	addi	%r1,%r1,80
+	blr
-- 
2.4.9 (Apple Git-60)

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 3/3] Move all the SHA1 implementations into one directory
  2015-11-05  6:38                                 ` [PATCH v4 3/3] Move all the SHA1 implementations into one directory atousa.p
@ 2015-11-05 18:29                                   ` Junio C Hamano
  0 siblings, 0 replies; 34+ messages in thread
From: Junio C Hamano @ 2015-11-05 18:29 UTC (permalink / raw)
  To: atousa.p; +Cc: git, Atousa Pahlevan Duprat

atousa.p@gmail.com writes:

> From: Atousa Pahlevan Duprat <apahlevan@ieee.org>
>
> The various SHA1 implementations were spread around in 3 directories.
> This makes it easier to understand what implementations are
> available at a glance.
>
> Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>
> ---

I am not strongly opposed to moving block and ppc (I am not strongly
for the movement, either, though).

I however think the chunked one should not be mixed together with
them--it is not a full SHA-1 hash implementation but belongs to a
different layer of abstration (and that is the reason why we
introduced a new layer of indirection in 1/3).

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 1/3] Provide another level of abstraction for the SHA1 utilities.
  2015-11-05  6:38                                 ` [PATCH v4 1/3] Provide another level of abstraction for the SHA1 utilities atousa.p
@ 2015-11-05 18:29                                   ` Junio C Hamano
  0 siblings, 0 replies; 34+ messages in thread
From: Junio C Hamano @ 2015-11-05 18:29 UTC (permalink / raw)
  To: atousa.p; +Cc: git, Atousa Pahlevan Duprat

atousa.p@gmail.com writes:

> From: Atousa Pahlevan Duprat <apahlevan@ieee.org>
>
> The git source uses git_SHA1_Update() and friends to call
> into the code that computes the hashes.  This is can then be
> mapped directly to an implementation that computes the hash,
> such as platform_SHA1_Update(); or as we will do in a subsequent
> patch, it can be mapped to something more complex that will in turn call
> into the platform's SHA implementation.
>
> Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>
> ---
>  cache.h | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index a9aaa03..a934a2e 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -12,10 +12,21 @@
>  
>  #include SHA1_HEADER
>  #ifndef git_SHA_CTX
> -#define git_SHA_CTX	SHA_CTX
> -#define git_SHA1_Init	SHA1_Init
> -#define git_SHA1_Update	SHA1_Update
> -#define git_SHA1_Final	SHA1_Final
> +
> +/* platform's underlying implementation of SHA1, could be OpenSSL,
> +   blk_SHA, Apple CommonCrypto, etc...  */
> +#define platform_SHA_CTX	SHA_CTX
> +#define platform_SHA1_Init	SHA1_Init
> +#define platform_SHA1_Update	SHA1_Update
> +#define platform_SHA1_Final    	SHA1_Final
> +
> +/* git may call platform's underlying implementation of SHA1 directly,
> +   or may call it through a wrapper */
> +#define git_SHA_CTX		platform_SHA_CTX
> +#define git_SHA1_Init		platform_SHA1_Init
> +#define git_SHA1_Update		platform_SHA1_Update
> +#define git_SHA1_Final		platform_SHA1_Final
> +
>  #endif
>  
>  #include <zlib.h>

This is not quite correct, I am afraid.  Our own implementations
still define git_SHA* macros, but they should be considered the
"platform" ones in the new world order with another level of
indirection.

I think the attached is closer to what we want.  The implementations
may give us platform_SHA*() in which case cache.h does not have to
give the fallback mapping from them to the OpenSSL compatible
interface used by OpenSSL and CommonCrypto.  Regardless of the
platform SHA-1 implementations, by default they are the ones used by
the rest of the system via git_SHA*().

And in the second step, git_SHA1_Update() may map to the Chunked
one, whose implementation would use platform_SHA1_Update().

-- >8 ---
From: Atousa Pahlevan Duprat <apahlevan@ieee.org>
Subject: sha1: provide another level of indirection for the SHA-1 functions

The git source uses git_SHA1_Update() and friends to call into the
code that computes the hashes.  Traditionally, we used to map these
directly to underlying implementation of the SHA-1 hash (e.g.
SHA1_Update() from OpenSSL or blk_SHA1_Update() from block-sha1/).

This arrangement however makes it hard to tweak behaviour of the
underlying implementation without fully replacing.  If we want to
introduce a tweaked_SHA1_Update() wrapper to implement the "Update"
in a slightly different way, for example, the implementation of the
wrapper still would want to call into the underlying implementation,
but tweaked_SHA1_Update() cannot call git_SHA1_Update() to get to
the underlying implementation (often but not always SHA1_Update()).

Add another level of indirection that maps platform_SHA1_Update()
and friends to their underlying implementations, and by default make
git_SHA1_Update() and friends map to platform_SHA1_* functions.

Doing it this way will later allow us to map git_SHA1_Update() to
tweaked_SHA1_Update(), and the latter can use platform_SHA1_Update()
in its implementation.

Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 block-sha1/sha1.h |  8 ++++----
 cache.h           | 22 +++++++++++++++++-----
 ppc/sha1.h        |  8 ++++----
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
index b864df6..4df6747 100644
--- a/block-sha1/sha1.h
+++ b/block-sha1/sha1.h
@@ -16,7 +16,7 @@ void blk_SHA1_Init(blk_SHA_CTX *ctx);
 void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *dataIn, unsigned long len);
 void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx);
 
-#define git_SHA_CTX	blk_SHA_CTX
-#define git_SHA1_Init	blk_SHA1_Init
-#define git_SHA1_Update	blk_SHA1_Update
-#define git_SHA1_Final	blk_SHA1_Final
+#define platform_SHA_CTX	blk_SHA_CTX
+#define platform_SHA1_Init	blk_SHA1_Init
+#define platform_SHA1_Update	blk_SHA1_Update
+#define platform_SHA1_Final	blk_SHA1_Final
diff --git a/cache.h b/cache.h
index a9aaa03..2f697c4 100644
--- a/cache.h
+++ b/cache.h
@@ -11,13 +11,25 @@
 #include "string-list.h"
 
 #include SHA1_HEADER
-#ifndef git_SHA_CTX
-#define git_SHA_CTX	SHA_CTX
-#define git_SHA1_Init	SHA1_Init
-#define git_SHA1_Update	SHA1_Update
-#define git_SHA1_Final	SHA1_Final
+#ifndef platform_SHA_CTX
+/*
+ * platform's underlying implementation of SHA-1; could be OpenSSL,
+ * blk_SHA, Apple CommonCrypto, etc...  Note that including
+ * SHA1_HEADER may have already defined platform_SHA_CTX for our
+ * own implementations like block-sha1 and ppc-sha1, so we list
+ * the default for OpenSSL compatible SHA-1 implementations here.
+ */
+#define platform_SHA_CTX	SHA_CTX
+#define platform_SHA1_Init	SHA1_Init
+#define platform_SHA1_Update	SHA1_Update
+#define platform_SHA1_Final    	SHA1_Final
 #endif
 
+#define git_SHA_CTX		platform_SHA_CTX
+#define git_SHA1_Init		platform_SHA1_Init
+#define git_SHA1_Update		platform_SHA1_Update
+#define git_SHA1_Final		platform_SHA1_Final
+
 #include <zlib.h>
 typedef struct git_zstream {
 	z_stream z;
diff --git a/ppc/sha1.h b/ppc/sha1.h
index c405f73..9b24b32 100644
--- a/ppc/sha1.h
+++ b/ppc/sha1.h
@@ -19,7 +19,7 @@ int ppc_SHA1_Init(ppc_SHA_CTX *c);
 int ppc_SHA1_Update(ppc_SHA_CTX *c, const void *p, unsigned long n);
 int ppc_SHA1_Final(unsigned char *hash, ppc_SHA_CTX *c);
 
-#define git_SHA_CTX	ppc_SHA_CTX
-#define git_SHA1_Init	ppc_SHA1_Init
-#define git_SHA1_Update	ppc_SHA1_Update
-#define git_SHA1_Final	ppc_SHA1_Final
+#define platform_SHA_CTX	ppc_SHA_CTX
+#define platform_SHA1_Init	ppc_SHA1_Init
+#define platform_SHA1_Update	ppc_SHA1_Update
+#define platform_SHA1_Final	ppc_SHA1_Final
-- 
2.6.2-535-ga9e37b0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/3] Limit the size of the data block passed to SHA1_Update()
  2015-11-05  6:38                                 ` [PATCH v4 2/3] Limit the size of the data block passed to SHA1_Update() atousa.p
@ 2015-11-05 18:29                                   ` Junio C Hamano
  2015-11-11 23:46                                     ` Atousa Duprat
  0 siblings, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2015-11-05 18:29 UTC (permalink / raw)
  To: atousa.p; +Cc: git, Atousa Pahlevan Duprat

atousa.p@gmail.com writes:

> +#ifndef git_SHA_CTX
> +
> +#ifdef SHA1_MAX_BLOCK_SIZE
> +#include "compat/sha1-chunked.h"
> +#define git_SHA_CTX		platform_SHA_CTX
> +#define git_SHA1_Init		platform_SHA1_Init
> +#define git_SHA1_Update		git_SHA1_Update_Chunked
> +#define git_SHA1_Final		platform_SHA1_Final
> +#else
>  #define git_SHA_CTX		platform_SHA_CTX
>  #define git_SHA1_Init		platform_SHA1_Init
>  #define git_SHA1_Update		platform_SHA1_Update
>  #define git_SHA1_Final		platform_SHA1_Final
> -
>  #endif
>  
> +#endif 
> +

Adjusting to the proposed change to 1/3, this step would become the
attached patch.  Note that I thought the above would not scale well
so I did it a bit differently.

-- >8 --
From: Atousa Pahlevan Duprat <apahlevan@ieee.org>
Subject: sha1: allow limiting the size of the data passed to SHA1_Update()

Using the previous commit's inredirection mechanism for SHA1,
support a chunked implementation of SHA1_Update() that limits the
amount of data in the chunk passed to SHA1_Update().

This is enabled by using the Makefile variable SHA1_MAX_BLOCK_SIZE
to specify chunk size.  When using Apple's CommonCrypto library this
is set to 1GiB (the implementation cannot handle more 4GiB).

Signed-off-by: Atousa Pahlevan Duprat <apahlevan@ieee.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile                     | 13 +++++++++++++
 cache.h                      |  6 ++++++
 compat/apple-common-crypto.h |  4 ++++
 compat/sha1-chunked.c        | 19 +++++++++++++++++++
 compat/sha1-chunked.h        |  2 ++
 5 files changed, 44 insertions(+)
 create mode 100644 compat/sha1-chunked.c
 create mode 100644 compat/sha1-chunked.h

diff --git a/Makefile b/Makefile
index 04c2231..6a4ca59 100644
--- a/Makefile
+++ b/Makefile
@@ -141,6 +141,10 @@ all::
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
+# Define SHA1_MAX_BLOCK_SIZE to limit the amount of data that will be hashed
+# in one call to the platform's SHA1_Update(). e.g. APPLE_COMMON_CRYPTO
+# wants 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined.
+#
 # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
 #
 # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
@@ -1335,6 +1339,11 @@ ifdef NO_POSIX_GOODIES
 	BASIC_CFLAGS += -DNO_POSIX_GOODIES
 endif
 
+ifdef APPLE_COMMON_CRYPTO
+	# Apple CommonCrypto requires chunking
+	SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
+endif
+
 ifdef BLK_SHA1
 	SHA1_HEADER = "block-sha1/sha1.h"
 	LIB_OBJS += block-sha1/sha1.o
@@ -1353,6 +1362,10 @@ endif
 endif
 endif
 
+ifdef SHA1_MAX_BLOCK_SIZE
+	LIB_OBJS += compat/sha1-chunked.o
+	BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
+endif
 ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif
diff --git a/cache.h b/cache.h
index 2f697c4..19c966d 100644
--- a/cache.h
+++ b/cache.h
@@ -30,6 +30,12 @@
 #define git_SHA1_Update		platform_SHA1_Update
 #define git_SHA1_Final		platform_SHA1_Final
 
+#ifdef SHA1_MAX_BLOCK_SIZE
+#include "compat/sha1-chunked.h"
+#undef git_SHA1_Update
+#define git_SHA1_Update		git_SHA1_Update_Chunked
+#endif
+
 #include <zlib.h>
 typedef struct git_zstream {
 	z_stream z;
diff --git a/compat/apple-common-crypto.h b/compat/apple-common-crypto.h
index c8b9b0e..d3fb264 100644
--- a/compat/apple-common-crypto.h
+++ b/compat/apple-common-crypto.h
@@ -16,6 +16,10 @@
 #undef TYPE_BOOL
 #endif
 
+#ifndef SHA1_MAX_BLOCK_SIZE
+#error Using Apple Common Crypto library requires setting SHA1_MAX_BLOCK_SIZE
+#endif
+
 #ifdef APPLE_LION_OR_NEWER
 #define git_CC_error_check(pattern, err) \
 	do { \
diff --git a/compat/sha1-chunked.c b/compat/sha1-chunked.c
new file mode 100644
index 0000000..6adfcfd
--- /dev/null
+++ b/compat/sha1-chunked.c
@@ -0,0 +1,19 @@
+#include "cache.h"
+
+int git_SHA1_Update_Chunked(platform_SHA_CTX *c, const void *data, size_t len)
+{
+	size_t nr;
+	size_t total = 0;
+	const char *cdata = (const char*)data;
+
+	while (len) {
+		nr = len;
+		if (nr > SHA1_MAX_BLOCK_SIZE)
+			nr = SHA1_MAX_BLOCK_SIZE;
+		platform_SHA1_Update(c, cdata, nr);
+		total += nr;
+		cdata += nr;
+		len -= nr;
+	}
+	return total;
+}
diff --git a/compat/sha1-chunked.h b/compat/sha1-chunked.h
new file mode 100644
index 0000000..7b2df28
--- /dev/null
+++ b/compat/sha1-chunked.h
@@ -0,0 +1,2 @@
+
+int git_SHA1_Update_Chunked(platform_SHA_CTX *c, const void *data, size_t len);
-- 
2.6.2-535-ga9e37b0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/3] Limit the size of the data block passed to SHA1_Update()
  2015-11-05 18:29                                   ` Junio C Hamano
@ 2015-11-11 23:46                                     ` Atousa Duprat
  0 siblings, 0 replies; 34+ messages in thread
From: Atousa Duprat @ 2015-11-11 23:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Atousa Pahlevan Duprat

> Adjusting to the proposed change to 1/3, this step would become the
> attached patch.  Note that I thought the above would not scale well
> so I did it a bit differently.

Thank you, I understand now the changes that you made.
Let me know if this can be improved any further.

Atousa

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2015-11-11 23:46 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-28 23:10 git fsck failure on OS X with files >= 4 GiB Rafael Espíndola
2015-10-29  6:46 ` Filipe Cabecinhas
     [not found] ` <CAEDE8505fXAwVXx=EZwxPHvXpMByzpnXJ9LBgfx3U6VUaFbPHw@mail.gmail.com>
2015-10-29 10:46   ` Rafael Espíndola
2015-10-29 15:15     ` Filipe Cabecinhas
2015-10-29 16:02       ` Atousa Duprat
2015-10-29 17:19         ` Junio C Hamano
2015-10-30  2:15           ` Atousa Duprat
2015-10-30 22:12             ` [PATCH] Limit the size of the data block passed to SHA1_Update() Atousa Pahlevan Duprat
2015-10-30 22:22               ` Junio C Hamano
2015-11-01  6:41                 ` Atousa Duprat
2015-11-01 18:31                   ` Junio C Hamano
2015-11-01  1:32               ` Eric Sunshine
2015-11-01  6:32                 ` atousa.p
2015-11-01  8:30                   ` Eric Sunshine
2015-11-01 18:37                   ` Junio C Hamano
2015-11-02 20:52                     ` Atousa Duprat
2015-11-02 21:21                       ` Junio C Hamano
2015-11-03  6:58                         ` [PATCH 1/2] " atousa.p
2015-11-03 11:51                           ` Torsten Bögershausen
2015-11-04  4:24                             ` [PATCH] " atousa.p
2015-11-04 19:51                               ` Eric Sunshine
2015-11-05  6:38                                 ` [PATCH v4 1/3] Provide another level of abstraction for the SHA1 utilities atousa.p
2015-11-05 18:29                                   ` Junio C Hamano
2015-11-05  6:38                                 ` [PATCH v4 2/3] Limit the size of the data block passed to SHA1_Update() atousa.p
2015-11-05 18:29                                   ` Junio C Hamano
2015-11-11 23:46                                     ` Atousa Duprat
2015-11-05  6:38                                 ` [PATCH v4 3/3] Move all the SHA1 implementations into one directory atousa.p
2015-11-05 18:29                                   ` Junio C Hamano
2015-11-04  4:27                             ` [PATCH 1/2] Limit the size of the data block passed to SHA1_Update() Atousa Duprat
2015-11-04 17:09                         ` [PATCH] " Junio C Hamano
2015-10-30 22:18             ` Atousa Pahlevan Duprat
2015-10-30 22:26               ` Randall S. Becker
2015-10-31 17:35                 ` Junio C Hamano
2015-11-01  6:37                 ` Atousa Duprat

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.