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

From: Jeff King <peff@peff.net>
Date: Oct 1, 2008, 03:47 UTC
Message-ID: <20081001034712.GE24513@coredump.intra.peff.net>
In-Reply-To: <20080930205122.GO21310@spearce.org>

On Tue, Sep 30, 2008 at 01:51:22PM -0700, Shawn O. Pearce wrote:

Show 6 quoted lines

> 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.. Right now, you are trying to find a way to squeak by because imap-send is the only thing that uses openssl, and it doesn't actually need our SHA definition. But any solution that exploits that characteristic is prone to failing later, when something _does_ need both of them.

You _could_ do something hack-ish like defining HEADER_SHA_H to avoid theirs being loaded, but:

  1. You are relying on their header guard never changing.

  2. This actually leaves room for subtle problems, since some code
     thinks a SHA_CTX is defined one way, and other code thinks it is
     defined another way. If one piece of code passes a SHA_CTX to the
     other (say another part of openssl), you risk invoking nasal
     demons.

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

making sure not to allow such a macro to be in effect when including openssl. This is similar to the way we override compat functions.

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.

-Peff