Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Abhishek Kumar <abhishekkumar8222@gmail.com>
To: nirmalhk7@gmail.com
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	dev+git@drbeat.li, git@vger.kernel.org, gitster@pobox.com
Subject: Re: Facing error in git-imap-send while compiling Git
Date: Mon, 17 Feb 2020 15:00:00 +0530
Message-ID: <CAHk66fvt-1RaLK8E7SDpocWM9OMAcA-gP5hjHq6r5N_FbATNgA@mail.gmail.com> (raw)

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

             reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-17  9:30 Abhishek Kumar [this message]
  -- strict thread matches above, loose matches on Subject: below --
2020-01-16 19:20 Nirmal Khedkar
2020-01-16 22:51 ` Junio C Hamano
2020-01-17 12:42   ` Nirmal Khedkar
2020-01-17 13:35     ` Johannes Schindelin
2020-01-20 19:12       ` Nirmal Khedkar
2020-01-20 21:35         ` Johannes Schindelin
2020-01-21 11:50           ` Nirmal Khedkar
2020-01-21 18:11             ` Junio C Hamano
2020-01-21 21:09             ` Johannes Schindelin
2020-01-22 19:20               ` Junio C Hamano
2020-01-30 20:26                 ` Nirmal Khedkar
2020-01-30 23:03                   ` Beat Bolli
2020-02-01 21:35                   ` Johannes Schindelin
2020-02-15 14:00                     ` Nirmal Khedkar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHk66fvt-1RaLK8E7SDpocWM9OMAcA-gP5hjHq6r5N_FbATNgA@mail.gmail.com \
    --to=abhishekkumar8222@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=dev+git@drbeat.li \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=nirmalhk7@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git