Atousa Duprat <atous...@gmail.com> writes:

> On Sun, Nov 1, 2015 at 10:37 AM, Junio C Hamano <gits...@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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to