On Wed, Sep 11, 2024 at 02:10:10AM -0400, Jeff King wrote:
> 2. That sets the options for _all_ versions of Git that we build. And
> it's possible for two versions to require conflicting knobs. E.g.,
> building with "make NO_OPENSSL=Nope OPENSSL_SHA1=Yes" causes
> imap-send.c to barf, because it declares a fallback typdef for SSL.
> This is something we may want to fix, but of course many historical
> versions are affected, and the interop scripts should be flexible
> enough to build everything.
And here's the fix to make this combo work (and likewise, the "fast"
variant). We'd still want the interop fix for the reasons given above,
but it feels like one less gotcha for people to hit if they are using
OPENSSL_SHA1_FAST.
-- >8 --
Subject: [PATCH] imap-send: handle NO_OPENSSL even when openssl exists
If NO_OPENSSL is defined, then imap-send.c defines a fallback "SSL"
type, which is just a void pointer that remains NULL. This works, but it
has one problem: it is using the type name "SSL", which conflicts with
the upstream name, if some other part of the system happens to include
openssl. For example:
$ make NO_OPENSSL=Nope OPENSSL_SHA1=Yes imap-send.o
CC imap-send.o
imap-send.c:35:15: error: conflicting types for ‘SSL’; have ‘void *’
35 | typedef void *SSL;
| ^~~
In file included from /usr/include/openssl/evp.h:26,
from sha1/openssl.h:4,
from hash.h:10,
from object.h:4,
from commit.h:4,
from refs.h:4,
from setup.h:4,
from imap-send.c:32:
/usr/include/openssl/types.h:187:23: note: previous declaration of ‘SSL’ with type ‘SSL’ {aka ‘struct ssl_st’}
187 | typedef struct ssl_st SSL;
| ^~~
make: *** [Makefile:2761: imap-send.o] Error 1
This is not a terribly common combination in practice:
1. Why are we disabling openssl support but still using its sha1? The
answer is that you may use the same build options across many
versions, and some older versions of Git no longer build with
modern versions of openssl.
2. Why are we using a totally unsafe sha1 that does not detect
collisions? You're right, we shouldn't. But in preparation for
using unsafe sha1 for non-cryptographic checksums, it would be nice
to be able to turn it on without hassle.
We can make this work by adjusting the way imap-send handles its
fallback. One solution is something like this:
#ifdef NO_OPENSSL
#define git_SSL void *
#else
#define git_SSL SSL
#endif
But we can observe that we only need this definition in one spot: the
struct which holds the variable. So rather than play around with macros
that may cause unexpected effects, we can just directly use the correct
type in that struct.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
imap-send.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index 2dd42807cd..ec68a06687 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -31,9 +31,6 @@
#include "parse-options.h"
#include "setup.h"
#include "strbuf.h"
-#if defined(NO_OPENSSL) && !defined(HAVE_OPENSSL_CSPRNG)
-typedef void *SSL;
-#endif
#ifdef USE_CURL_FOR_IMAP_SEND
#include "http.h"
#endif
@@ -85,7 +82,11 @@ struct imap_server_conf {
struct imap_socket {
int fd[2];
+#if defined(NO_OPENSSL) && !defined(HAVE_OPENSSL_CSPRNG)
+ void *ssl;
+#else
SSL *ssl;
+#endif
};
struct imap_buffer {
--
2.46.0.883.g5805d96482
