Re: Facing error in git-imap-send while compiling Git

[Abhishek Kumar <abhishekkumar8222@gmail.com>]

Greetings Nirmal

> what imap-send does (in words simpler than ones in manpages).

IMAP is a protocol that stores email messages on a mail server but
allows end-user to view and manipulate them as local files.

imap-send sends a collection of patches from stdin to an IMAP folder.
You create patches using `git format-patch`, edit them and once
satisfied, send them to your drafts folder using `git imap-send`.

**Note**: This does not mean that they are sent out, but rather they
are (usually) in your email service's draft folder.

As for your diff, I have some suggestions -
---
diff --git a/imap-send.c b/imap-send.c
index 6c54d8c29d..3248bc2123 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -41,7 +41,9 @@ typedef void *SSL;
 /* We don't have curl, so continue to use the historical implementation */
 #define USE_CURL_DEFAULT 0
 #endif
-
> +#ifndef SSL_library_init
> +       #define SSL_library_init();
> +#endif

If the macro does not exist, you define it to - nothing, really. You
might have meant this. [1]
```
#ifndef SSL_libary_init
#define SSL_library_init() OPENSSL_init_ssl(0, null)
#endif
```
Regardless, you do check below for the version (hence indirectly
whether the macro exists). You can safely remove these lines.

 static int ssl_socket_connect(struct imap_socket *sock, int
use_tls_only, int verify)
 {
-#if (OPENSSL_VERSION_NUMBER >= 0x10000000L)
-       const SSL_METHOD *meth;
-#else
-       SSL_METHOD *meth;
-#endif
-       SSL_CTX *ctx;
-       int ret;
-       X509 *cert;
-
-       SSL_library_init();
-       SSL_load_error_strings();
> +       #if (OPENSSL_VERSION_NUMBER >= 0x10000000L)
> +               const SSL_METHOD *meth;
> +       #else
> +               SSL_METHOD *meth;
> +       #endif

Maintain zero indentation for macros for consistency.
Also define `METHOD_NAME` as `SSLv23_method` and `TLS_method` to
reduce noise below.

+               SSL_CTX *ctx;
+               int ret;
+               X509 *cert;
+
> +       #if OPENSSL_VERSION_NUMBER >= 0x10100000L ||
> + defined(LIBRESSL_VERSION_NUMBER)
> +               OPENSSL_init_ssl(0, NULL);

This will be executed if openssl version >= 1.1 or has libressl
installed - which means it will *also* be executed for
openssl < 1.1 and libressl installed. OPENSSL_init_ssl hasn't been
defined for older versions and will throw an undefined reference as well. [2]
+               meth = TLS_method();
+       #else
+               SSL_library_init();
+               SSL_load_error_strings();
+               meth = SSLv23_method();
+       #endif

-       meth = SSLv23_method();
        if (!meth) {
-               ssl_socket_perror("SSLv23_method");
> +       #if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
> +                       ssl_socket_perror("TLS_method");
> +       #else
> +                       ssl_socket_perror("SSLv23_method");
> +       #endif

Use `METHOD_NAME` defined above.

                return -1;
        }

As a general note for this patch - I would rather make OpenSSL 1.1 the
common case, using directives to make it compatible with the older
versions. Do consult with Junio and Johannes over the larger picture
here.
---

Regards
Abhishek

[1]: https://github.com/openssl/openssl/blob/8083fd3a183d4c881d6b15727cbc6cb7faeb3280/include/openssl/ssl.h#L1995
[2]: https://www.openssl.org/docs/man1.1.0/man3/OPENSSL_init_ssl.html