From: Junio C Hamano <gitster@pobox•com>
To: Atousa Pahlevan Duprat <atousa.p@gmail•com>
Cc: git@vger•kernel.org, Atousa Pahlevan Duprat <apahlevan@ieee•org>
Subject: Re: [PATCH] Limit the size of the data block passed to SHA1_Update()
Date: Fri, 30 Oct 2015 15:22:05 -0700 [thread overview]
Message-ID: <xmqqa8r0j8tu.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1446243122-21464-1-git-send-email-apahlevan@ieee.org> (Atousa Pahlevan Duprat's message of "Fri, 30 Oct 2015 15:12:02 -0700")
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.
next prev parent reply other threads:[~2015-10-30 22:22 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqa8r0j8tu.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=apahlevan@ieee$(echo .)org \
--cc=atousa.p@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox