Re: [PATCH, resent] fix openssl headers conflicting with custom SHA1 implementations

From: Shawn O. Pearce <spearce@spearce.org>
Date: Oct 1, 2008, 15:54 UTC
Message-ID: <20081001155458.GF21310@spearce.org>
In-Reply-To: <20081001034712.GE24513@coredump.intra.peff.net>

Jeff King <peff@peff.net> wrote:

Show 11 quoted lines

> On Tue, Sep 30, 2008 at 01:51:22PM -0700, Shawn O. Pearce wrote:
> 
> > Yea, its a bit ugly due to the rats nest of system includes.
> > Right now I don't see how we can include your patch, it breaks a
> > major platform for us.  But obviously my "fix" is also bogus and
> > won't get ARM working again.
> > 
> > Any other ideas we can try?  'cause I don't have any right now.  :-|
> 
> I think you have an inherent conflict. Using openssl is going to end up
> including their SHA definition, and we clearly can't include both..

...

Show 8 quoted lines

> So I think the right way is probably to use a level of indirection. Turn
> the ARM implementation into
> 
>   void ARM_SHA1_Init()
> 
> and
> 
>   #define SHA1_Init ARM_SHA1_Init

Ick.  I agree that renaming our SHA1 implementation to a different
set of symbols is the right solution.  But this could cause trouble
in any code that needs both SHA1 implementation and OpenSSL,
especially if a SHA_CTX was passed to a function in another module.

> You can make it even simpler by just having all code call git_SHA1_Init,
> and that will expand to whichever implementation has been chosen.

 $ git grep --cached SHA1_Init | wc -l
 32
 $ git grep --cached SHA1_Final | wc -l
 26
 $ git grep --cached SHA1_Update | wc -l
 39
 $ git grep --cached SHA_CTX | wc -l
 48

I think its easy enough to just rename our SHA_CTX and SHA_* functions to something more git specific. Since its mostly a global s/// we should be able to run it through into master without too much effort. It would however make merging certain changes from maint fun, but I think we can handle it until the 1.6.1 release goes final. We tend not to twiddle SHA1 related code in maint.

Of course this is me speaking... Junio is the one that would be faced with the brunt of that pain. If it comes down to it I'd be willing to handle the maint branch and its merges back into master for him.

A funny thing from the above stats... we have 6 SHA1_Init calls which aren't balanced by a SHA1_Final call. Weird.

-- 
Shawn.