git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>,
	Jeff King <peff@peff.net>
Cc: Jonathan Nieder <jrnieder@gmail.com>, git@vger.kernel.org
Subject: Re: [RFC PATCH] Move SHA-1 implementation selection into a header file
Date: Tue, 14 Mar 2017 13:44:48 -0700	[thread overview]
Message-ID: <xmqq1stzio5b.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170314201424.vccij5z2ortq4a4o@sigill.intra.peff.net> (Jeff King's message of "Tue, 14 Mar 2017 16:14:24 -0400")

OK, then I'll queue this.  The selection still goes to BASIC_CFLAGS
so the dependencies for re-compilation should be right, I'd think.

-- >8 --
From: "brian m. carlson" <sandals@crustytoothpaste.net>
Date: Sat, 11 Mar 2017 22:28:18 +0000
Subject: [PATCH] hash.h: move SHA-1 implementation selection into a header file

Many developers use functionality in their editors that allows for quick
syntax checks, including warning about questionable constructs.  This
functionality allows rapid development with fewer errors.  However, such
functionality generally does not allow the specification of
project-specific defines or command-line options.

Since the SHA1_HEADER include is not defined in such a case, developers
see spurious errors when using these tools.  Furthermore, while using a
macro as the argument to #include is permitted by C11, it isn't
permitted by C89 and C99, and there are known implementations which
reject it.

Instead of using SHA1_HEADER, create a hash.h header and use #if
and #elif to select the desired header.  Have the Makefile pass an
appropriate option to help the header select the right implementation to
use.

[jc: make BLK_SHA1 the fallback default as discussed on list,
e.g. <20170314201424.vccij5z2ortq4a4o@sigill.intra.peff.net>; also
remove SHA1_HEADER and SHA1_HEADER_SQ that are no longer used].

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile | 12 +++++-------
 cache.h  |  2 +-
 hash.h   | 14 ++++++++++++++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 8e4081e061..25c21f08b1 100644
--- a/Makefile
+++ b/Makefile
@@ -1387,19 +1387,19 @@ ifdef APPLE_COMMON_CRYPTO
 endif
 
 ifdef BLK_SHA1
-	SHA1_HEADER = "block-sha1/sha1.h"
 	LIB_OBJS += block-sha1/sha1.o
+	BASIC_CFLAGS += -DSHA1_BLK
 else
 ifdef PPC_SHA1
-	SHA1_HEADER = "ppc/sha1.h"
 	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
+	BASIC_CFLAGS += -DSHA1_PPC
 else
 ifdef APPLE_COMMON_CRYPTO
 	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
-	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
+	BASIC_CFLAGS += -DSHA1_APPLE
 else
-	SHA1_HEADER = <openssl/sha.h>
 	EXTLIBS += $(LIB_4_CRYPTO)
+	BASIC_CFLAGS += -DSHA1_OPENSSL
 endif
 endif
 endif
@@ -1591,7 +1591,6 @@ endif
 
 # Shell quote (do not use $(call) to accommodate ancient setups);
 
-SHA1_HEADER_SQ = $(subst ','\'',$(SHA1_HEADER))
 ETC_GITCONFIG_SQ = $(subst ','\'',$(ETC_GITCONFIG))
 ETC_GITATTRIBUTES_SQ = $(subst ','\'',$(ETC_GITATTRIBUTES))
 
@@ -1624,8 +1623,7 @@ PERLLIB_EXTRA_SQ = $(subst ','\'',$(PERLLIB_EXTRA))
 # from the dependency list, that would make each entry appear twice.
 LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)
 
-BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
-	$(COMPAT_CFLAGS)
+BASIC_CFLAGS += $(COMPAT_CFLAGS)
 LIB_OBJS += $(COMPAT_OBJS)
 
 # Quote for C
diff --git a/cache.h b/cache.h
index 61fc86e6d7..f98c95bf2a 100644
--- a/cache.h
+++ b/cache.h
@@ -10,8 +10,8 @@
 #include "trace.h"
 #include "string-list.h"
 #include "pack-revindex.h"
+#include "hash.h"
 
-#include SHA1_HEADER
 #ifndef platform_SHA_CTX
 /*
  * platform's underlying implementation of SHA-1; could be OpenSSL,
diff --git a/hash.h b/hash.h
new file mode 100644
index 0000000000..f0d9ddd0c2
--- /dev/null
+++ b/hash.h
@@ -0,0 +1,14 @@
+#ifndef HASH_H
+#define HASH_H
+
+#if defined(SHA1_PPC)
+#include "ppc/sha1.h"
+#elif defined(SHA1_APPLE)
+#include <CommonCrypto/CommonDigest.h>
+#elif defined(SHA1_OPENSSL)
+#include <openssl/sha.h>
+#else /* SHA1_BLK */
+#include "block-sha1/sha1.h"
+#endif
+
+#endif

  reply	other threads:[~2017-03-14 20:44 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-11 22:28 [RFC PATCH] Move SHA-1 implementation selection into a header file brian m. carlson
2017-03-12 13:01 ` Jeff King
2017-03-12 16:51   ` brian m. carlson
2017-03-12 20:12     ` Jeff King
2017-03-12 17:54   ` Junio C Hamano
2017-03-14 18:41 ` Jonathan Nieder
2017-03-14 20:14   ` Jeff King
2017-03-14 20:44     ` Junio C Hamano [this message]
2017-03-14 21:26       ` Jeff King
2017-03-14 21:50       ` Jonathan Nieder
2017-03-14 23:42       ` Ramsay Jones
2017-03-14 23:46         ` brian m. carlson
2017-03-15  0:15           ` Ramsay Jones
2017-03-15 15:57             ` Junio C Hamano
2017-03-15 19:48               ` Ramsay Jones
2017-03-14 21:56     ` Jonathan Nieder

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqq1stzio5b.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).