[PATCH 1/3] configure.ac: check 'tv_nsec' field in 'struct stat'

[PATCH 1/3] configure.ac: check 'tv_nsec' field in 'struct stat'

[Reuben Hawkins]

Detect 'tv_nsec' field in 'struct stat' and set Makefile variable
NO_NSEC appropriately.

A side-effect of the above detection is that we also determine
whether 'stat.st_mtimespec' is available, so, as a bonus, set the
Makefile variable USE_ST_TIMESPEC, as well.

Signed-off-by: Reuben Hawkins <reubenhwk@gmail.com>
---
 configure.ac | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/configure.ac b/configure.ac
index 6af9647..210eb4e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -754,6 +754,19 @@ AC_CHECK_TYPES([struct itimerval],
 [#include <sys/time.h>])
 GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])
 #
+# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exists.
+# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor
+# stat.st_mtimespec.tv_nsec exists.
+AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec])
+AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])
+if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then
+	USE_ST_TIMESPEC=YesPlease
+	GIT_CONF_SUBST([USE_ST_TIMESPEC])
+elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then
+	NO_NSEC=YesPlease
+	GIT_CONF_SUBST([NO_NSEC])
+fi
+#
 # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
 AC_CHECK_MEMBER(struct dirent.d_ino,
 [NO_D_INO_IN_DIRENT=],
-- 
2.2.0.68.g8f72f0c.dirty

[PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC

[Reuben Hawkins]

The checks will override and unset YesPlease settings for HAVE_CLOCK_GETTIME
and HAVE_CLOCK_MONOTONIC in config.mak.uname.

CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3 systems
being used in production.

Signed-off-by: Reuben Hawkins <reubenhwk@gmail.com>
---
 Makefile         |  6 ++++++
 config.mak.uname |  1 +
 configure.ac     | 22 ++++++++++++++++++++++
 trace.c          |  2 +-
 4 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 7482a4d..57e33f2 100644
--- a/Makefile
+++ b/Makefile
@@ -339,6 +339,8 @@ all::
 # return NULL when it receives a bogus time_t.
 #
 # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime in librt.
+#
+# Define HAVE_CLOCK_MONOTONIC if your platform has CLOCK_MONOTONIC in librt.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1382,6 +1384,10 @@ ifdef HAVE_CLOCK_GETTIME
 	EXTLIBS += -lrt
 endif
 
+ifdef HAVE_CLOCK_MONOTONIC
+	BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC
+endif
+
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK = NoThanks
 endif
diff --git a/config.mak.uname b/config.mak.uname
index a2f380f..926773e 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -35,6 +35,7 @@ ifeq ($(uname_S),Linux)
 	LIBC_CONTAINS_LIBINTL = YesPlease
 	HAVE_DEV_TTY = YesPlease
 	HAVE_CLOCK_GETTIME = YesPlease
+	HAVE_CLOCK_MONOTONIC = YesPlease
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
 	HAVE_ALLOCA_H = YesPlease
diff --git a/configure.ac b/configure.ac
index 210eb4e..c3293b9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -924,6 +924,28 @@ AC_CHECK_LIB([iconv], [locale_charset],
                      [CHARSET_LIB=-lcharset])])
 GIT_CONF_SUBST([CHARSET_LIB])
 #
+# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
+GIT_CHECK_FUNC(clock_gettime,
+	[HAVE_CLOCK_GETTIME=YesPlease],
+	[HAVE_CLOCK_GETTIME=])
+GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])
+
+AC_DEFUN([CLOCK_MONOTONIC_SRC], [
+AC_LANG_PROGRAM([[
+#include <time.h>
+clockid_t id = CLOCK_MONOTONIC;
+]])])
+
+#
+# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.
+AC_MSG_CHECKING([for CLOCK_MONOTONIC])
+AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
+	[AC_MSG_RESULT([yes])
+	HAVE_CLOCK_MONOTONIC=YesPlease],
+	[AC_MSG_RESULT([no])
+	HAVE_CLOCK_MONOTONIC=])
+GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])
+#
 # Define NO_SETITIMER if you don't have setitimer.
 GIT_CHECK_FUNC(setitimer,
 [NO_SETITIMER=],
diff --git a/trace.c b/trace.c
index 4778608..bfbd48f 100644
--- a/trace.c
+++ b/trace.c
@@ -324,7 +324,7 @@ int trace_want(struct trace_key *key)
 	return !!get_trace_fd(key);
 }
 
-#ifdef HAVE_CLOCK_GETTIME
+#if defined(HAVE_CLOCK_GETTIME) && defined(HAVE_CLOCK_MONOTONIC)
 
 static inline uint64_t highres_nanos(void)
 {
-- 
2.2.0.68.g8f72f0c.dirty

Re: [PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC

[Eric Sunshine]

On Thu, Jan 8, 2015 at 3:00 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:
> The checks will override and unset YesPlease settings for HAVE_CLOCK_GETTIME
> and HAVE_CLOCK_MONOTONIC in config.mak.uname.

I find this hard to grok and would rewrite it as:

    Set or clear Makefile variables HAVE_CLOCK_GETTIME and
    HAVE_CLOCK_MONOTONIC based upon results of the checks (overriding
    default values from config.mak.uname).

> CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3 systems
> being used in production.
>
> Signed-off-by: Reuben Hawkins <reubenhwk@gmail.com>

With or without the minor rewrite above...

Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

> ---
> diff --git a/Makefile b/Makefile
> index 7482a4d..57e33f2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -339,6 +339,8 @@ all::
>  # return NULL when it receives a bogus time_t.
>  #
>  # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime in librt.
> +#
> +# Define HAVE_CLOCK_MONOTONIC if your platform has CLOCK_MONOTONIC in librt.
>
>  GIT-VERSION-FILE: FORCE
>         @$(SHELL_PATH) ./GIT-VERSION-GEN
> @@ -1382,6 +1384,10 @@ ifdef HAVE_CLOCK_GETTIME
>         EXTLIBS += -lrt
>  endif
>
> +ifdef HAVE_CLOCK_MONOTONIC
> +       BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC
> +endif
> +
>  ifeq ($(TCLTK_PATH),)
>  NO_TCLTK = NoThanks
>  endif
> diff --git a/config.mak.uname b/config.mak.uname
> index a2f380f..926773e 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -35,6 +35,7 @@ ifeq ($(uname_S),Linux)
>         LIBC_CONTAINS_LIBINTL = YesPlease
>         HAVE_DEV_TTY = YesPlease
>         HAVE_CLOCK_GETTIME = YesPlease
> +       HAVE_CLOCK_MONOTONIC = YesPlease
>  endif
>  ifeq ($(uname_S),GNU/kFreeBSD)
>         HAVE_ALLOCA_H = YesPlease
> diff --git a/configure.ac b/configure.ac
> index 210eb4e..c3293b9 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -924,6 +924,28 @@ AC_CHECK_LIB([iconv], [locale_charset],
>                       [CHARSET_LIB=-lcharset])])
>  GIT_CONF_SUBST([CHARSET_LIB])
>  #
> +# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
> +GIT_CHECK_FUNC(clock_gettime,
> +       [HAVE_CLOCK_GETTIME=YesPlease],
> +       [HAVE_CLOCK_GETTIME=])
> +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])
> +
> +AC_DEFUN([CLOCK_MONOTONIC_SRC], [
> +AC_LANG_PROGRAM([[
> +#include <time.h>
> +clockid_t id = CLOCK_MONOTONIC;
> +]])])
> +
> +#
> +# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.
> +AC_MSG_CHECKING([for CLOCK_MONOTONIC])
> +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
> +       [AC_MSG_RESULT([yes])
> +       HAVE_CLOCK_MONOTONIC=YesPlease],
> +       [AC_MSG_RESULT([no])
> +       HAVE_CLOCK_MONOTONIC=])
> +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])
> +#
>  # Define NO_SETITIMER if you don't have setitimer.
>  GIT_CHECK_FUNC(setitimer,
>  [NO_SETITIMER=],
> diff --git a/trace.c b/trace.c
> index 4778608..bfbd48f 100644
> --- a/trace.c
> +++ b/trace.c
> @@ -324,7 +324,7 @@ int trace_want(struct trace_key *key)
>         return !!get_trace_fd(key);
>  }
>
> -#ifdef HAVE_CLOCK_GETTIME
> +#if defined(HAVE_CLOCK_GETTIME) && defined(HAVE_CLOCK_MONOTONIC)
>
>  static inline uint64_t highres_nanos(void)
>  {
> --
> 2.2.0.68.g8f72f0c.dirty

[PATCH 3/3] configure.ac: check for HMAC_CTX_cleanup

[Reuben Hawkins]

OpenSSL version 0.9.6b and before defined the function HMAC_cleanup.
Newer versions define HMAC_CTX_cleanup.  Check for HMAC_CTX_cleanup and
fall back to HMAC_cleanup when the newer function is missing.

Signed-off-by: Reuben Hawkins <reubenhwk@gmail.com>
---
 Makefile          | 6 ++++++
 configure.ac      | 4 ++++
 git-compat-util.h | 3 +++
 3 files changed, 13 insertions(+)

diff --git a/Makefile b/Makefile
index 57e33f2..2ce1f1f 100644
--- a/Makefile
+++ b/Makefile
@@ -341,6 +341,9 @@ all::
 # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime in librt.
 #
 # Define HAVE_CLOCK_MONOTONIC if your platform has CLOCK_MONOTONIC in librt.
+#
+# Define NO_HMAC_CTX_CLEANUP if your OpenSSL is version 0.9.6b or earlier to
+# cleanup the HMAC context with the older HMAC_cleanup function.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1061,6 +1064,9 @@ ifndef NO_OPENSSL
 	ifdef NEEDS_CRYPTO_WITH_SSL
 		OPENSSL_LIBSSL += -lcrypto
 	endif
+	ifdef NO_HMAC_CTX_CLEANUP
+		BASIC_CFLAGS += -DNO_HMAC_CTX_CLEANUP
+	endif
 else
 	BASIC_CFLAGS += -DNO_OPENSSL
 	BLK_SHA1 = 1
diff --git a/configure.ac b/configure.ac
index c3293b9..9c66c3e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -924,6 +924,10 @@ AC_CHECK_LIB([iconv], [locale_charset],
                      [CHARSET_LIB=-lcharset])])
 GIT_CONF_SUBST([CHARSET_LIB])
 #
+# Define NO_HMAC_CTX_CLEANUP=YesPlease if HMAC_CTX_cleanup is missing.
+AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup],
+	[], [GIT_CONF_SUBST([NO_HMAC_CTX_CLEANUP], [YesPlease])])
+#
 # Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
 GIT_CHECK_FUNC(clock_gettime,
 	[HAVE_CLOCK_GETTIME=YesPlease],
diff --git a/git-compat-util.h b/git-compat-util.h
index 400e921..2fdca2d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -213,6 +213,9 @@ extern char *gitbasename(char *);
 #ifndef NO_OPENSSL
 #include <openssl/ssl.h>
 #include <openssl/err.h>
+#ifdef NO_HMAC_CTX_CLEANUP
+#define HMAC_CTX_cleanup HMAC_cleanup
+#endif
 #endif
 
 /* On most systems <netdb.h> would have given us this, but
-- 
2.2.0.68.g8f72f0c.dirty

Re: [PATCH 3/3] configure.ac: check for HMAC_CTX_cleanup

[Eric Sunshine]

On Thu, Jan 8, 2015 at 3:00 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:
> OpenSSL version 0.9.6b and before defined the function HMAC_cleanup.
> Newer versions define HMAC_CTX_cleanup.  Check for HMAC_CTX_cleanup and
> fall back to HMAC_cleanup when the newer function is missing.
>
> Signed-off-by: Reuben Hawkins <reubenhwk@gmail.com>

Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

Note that this patch has a minor and simple-to-resolve conflict with
b195aa00c1 (git-compat-util: suppress unavoidable Apple-specific
deprecation warnings; 2014-12-16) which was just promoted to master.
Junio may resolve it locally or ask you to re-send.

> ---
> diff --git a/Makefile b/Makefile
> index 57e33f2..2ce1f1f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -341,6 +341,9 @@ all::
>  # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime in librt.
>  #
>  # Define HAVE_CLOCK_MONOTONIC if your platform has CLOCK_MONOTONIC in librt.
> +#
> +# Define NO_HMAC_CTX_CLEANUP if your OpenSSL is version 0.9.6b or earlier to
> +# cleanup the HMAC context with the older HMAC_cleanup function.
>
>  GIT-VERSION-FILE: FORCE
>         @$(SHELL_PATH) ./GIT-VERSION-GEN
> @@ -1061,6 +1064,9 @@ ifndef NO_OPENSSL
>         ifdef NEEDS_CRYPTO_WITH_SSL
>                 OPENSSL_LIBSSL += -lcrypto
>         endif
> +       ifdef NO_HMAC_CTX_CLEANUP
> +               BASIC_CFLAGS += -DNO_HMAC_CTX_CLEANUP
> +       endif
>  else
>         BASIC_CFLAGS += -DNO_OPENSSL
>         BLK_SHA1 = 1
> diff --git a/configure.ac b/configure.ac
> index c3293b9..9c66c3e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -924,6 +924,10 @@ AC_CHECK_LIB([iconv], [locale_charset],
>                       [CHARSET_LIB=-lcharset])])
>  GIT_CONF_SUBST([CHARSET_LIB])
>  #
> +# Define NO_HMAC_CTX_CLEANUP=YesPlease if HMAC_CTX_cleanup is missing.
> +AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup],
> +       [], [GIT_CONF_SUBST([NO_HMAC_CTX_CLEANUP], [YesPlease])])
> +#
>  # Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
>  GIT_CHECK_FUNC(clock_gettime,
>         [HAVE_CLOCK_GETTIME=YesPlease],
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 400e921..2fdca2d 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -213,6 +213,9 @@ extern char *gitbasename(char *);
>  #ifndef NO_OPENSSL
>  #include <openssl/ssl.h>
>  #include <openssl/err.h>
> +#ifdef NO_HMAC_CTX_CLEANUP
> +#define HMAC_CTX_cleanup HMAC_cleanup
> +#endif
>  #endif
>
>  /* On most systems <netdb.h> would have given us this, but
> --
> 2.2.0.68.g8f72f0c.dirty

Re: [PATCH 1/3] configure.ac: check 'tv_nsec' field in 'struct stat'

[Eric Sunshine]

On Thu, Jan 8, 2015 at 3:00 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:
> Detect 'tv_nsec' field in 'struct stat' and set Makefile variable
> NO_NSEC appropriately.
>
> A side-effect of the above detection is that we also determine
> whether 'stat.st_mtimespec' is available, so, as a bonus, set the
> Makefile variable USE_ST_TIMESPEC, as well.
>
> Signed-off-by: Reuben Hawkins <reubenhwk@gmail.com>

These patches may or may not deserve a Helped-by:.

With or without the Helped-by: and the minor style fix-up below...

Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

Thanks for the perseverance.

For convenience of other reviewers: This is v3. v1 through v3 are
threaded together here:
http://thread.gmane.org/gmane.comp.version-control.git/261619

> ---
> diff --git a/configure.ac b/configure.ac
> index 6af9647..210eb4e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -754,6 +754,19 @@ AC_CHECK_TYPES([struct itimerval],
>  [#include <sys/time.h>])
>  GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])
>  #
> +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exists.
> +# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor
> +# stat.st_mtimespec.tv_nsec exists.
> +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec])
> +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])
> +if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then

Style: For consistency with all other 'if' conditionals in
configure.ac, drop the space before the semicolon.

> +       USE_ST_TIMESPEC=YesPlease
> +       GIT_CONF_SUBST([USE_ST_TIMESPEC])
> +elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then

Ditto.

> +       NO_NSEC=YesPlease
> +       GIT_CONF_SUBST([NO_NSEC])
> +fi
> +#
>  # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
>  AC_CHECK_MEMBER(struct dirent.d_ino,
>  [NO_D_INO_IN_DIRENT=],
> --
> 2.2.0.68.g8f72f0c.dirty

Re: [PATCH 3/3] configure.ac,imap-send.c: check HMAC_CTX_cleanup

[Eric Sunshine]

On Sun, Dec 21, 2014 at 1:53 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:
> Older versions of OpenSSL have HMAC_cleanup, but not HMAC_CTX_cleanup.
> This change checks for both, uses HMAC_CTX_cleanup on platforms which
> have it, otherwise falls back to HMAC_cleanup.

On this project, imperative mood is preferred. Also, this _is_ a patch
(implying "change"), so it's not necessary to say "This change". An
alternate way to write the above might be:

    Check for both, and use HMAC_CTX_cleanup when available,
    otherwise HMAC_cleanup.

> This changes makes building GIT on older platforms less tedious.

This is implied by the first paragraph, so no need to state it here explicitly.

More below.

> ---
> diff --git a/Makefile b/Makefile
> index 7482a4d..a495d94 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1059,6 +1059,12 @@ ifndef NO_OPENSSL
>         ifdef NEEDS_CRYPTO_WITH_SSL
>                 OPENSSL_LIBSSL += -lcrypto
>         endif
> +       ifdef HAVE_HMAC_CTX_CLEANUP
> +               BASIC_CFLAGS += -DHAVE_HMAC_CTX_CLEANUP
> +       endif
> +       ifdef HAVE_HMAC_CLEANUP
> +               BASIC_CFLAGS += -DHAVE_HMAC_CLEANUP
> +       endif

See discussion below regarding build breakage due to this incomplete change.

>  else
>         BASIC_CFLAGS += -DNO_OPENSSL
>         BLK_SHA1 = 1
> diff --git a/configure.ac b/configure.ac
> index 3900044..b22788c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -899,6 +899,7 @@ GIT_CONF_SUBST([SNPRINTF_RETURNS_BOGUS])
>  ## Checks for library functions.
>  ## (in default C library and libraries checked by AC_CHECK_LIB)
>  AC_MSG_NOTICE([CHECKS for library functions])
> +

Sneaking in unrelated whitespace change.

>  #
>  # Define NO_LIBGEN_H if you don't have libgen.h.
>  AC_CHECK_HEADER([libgen.h],
> @@ -932,6 +933,18 @@ AC_CHECK_LIB([iconv], [locale_charset],
>                       [CHARSET_LIB=-lcharset])])
>  GIT_CONF_SUBST([CHARSET_LIB])
>  #
> +# Define HAVE_HMAC_CTX_CLEANUP=Yes if we have the newer HMAC cleanup function

s/Yes/YesPease/
s/$/./

> +AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup],
> +       [HAVE_HMAC_CTX_CLEANUP=Yes],
> +       [], [])

No need to pass trailing empty [] arguments explicitly in m4.

> +GIT_CONF_SUBST([HAVE_HMAC_CTX_CLEANUP])

You need to document this new variable in the comment block at the top
of Makefile so that people who build without running the configure
script will know that they may need to tweak it.

> +#
> +# Define HAVE_HMAC_CLEANUP=Yes if we have the older HMAC cleanup function

s/Yes/YesPease/
s/$/./

> +AC_CHECK_LIB([crypto], [HMAC_cleanup],
> +       [HAVE_HMAC_CLEANUP=Yes],
> +       [], [])
> +GIT_CONF_SUBST([HAVE_HMAC_CLEANUP])

This test for HMAC_cleanup() is likely overkill until we encounter a
system which has neither HMAC_CTX_cleanup() nor HMAC_cleanup(). For
the present, it's probably safe to assume that the older
HMAC_cleanup() is available everywhere when HMAC_CTX_cleanup() is not.

> +#
>  # Define NO_CLOCK_GETTIME if you don't have clock_gettime.
>  GIT_CHECK_FUNC(clock_gettime,
>  [HAVE_CLOCK_GETTIME=Yes],
> diff --git a/imap-send.c b/imap-send.c
> index 70bcc7a..eec2378 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -861,7 +861,13 @@ static char *cram(const char *challenge_64, const char *user, const char *pass)
>         HMAC_Init(&hmac, (unsigned char *)pass, strlen(pass), EVP_md5());
>         HMAC_Update(&hmac, (unsigned char *)challenge, decoded_len);
>         HMAC_Final(&hmac, hash, NULL);
> +#if defined(HAVE_HMAC_CTX_CLEANUP)
>         HMAC_CTX_cleanup(&hmac);
> +#elif defined(HAVE_HMAC_CLEANUP)
> +       HMAC_cleanup(&hmac);
> +#else
> +# error "no HMAC_cleanup function"
> +#endif

Several issues:

This change utterly breaks the build for people who do not run the
configure script since neither HAVE_HMAC_CTX_CLEANUP nor
HAVE_HMAC_CLEANUP Makefile variable is set (which means that the
corresponding C #defines are never made).

Somehow, you need to make sure that Makefile variable
HAVE_HMAC_CTX_CLEANUP is set by default since HMAC_CTX_cleanup() is
available on most platforms, and only a few much older ones lack it.
Typically, you would do this in config.make.uname, however, this is
one of those instances in which it is easier to disable a feature for
the rare case than to enable it for the common case. Therefore, it
would be more appropriate to name this variable NO_HMAC_CTX_CLEANUP
(and document it appropriately in Makefile).

Finally, rather than polluting imap-send.c with #if conditionals,
relegate the ugliness to git-compat-util.h. For instance, within the
'#ifndef NO_OPENSSL' conditional, you might add:

    #ifdef NO_HMAC_CTX_CLEANUP
    #define HMAC_CTX_cleanup HMAC_cleanup
    #endif

and then you don't have to touch imap-send.c at all.

>         hex[32] = 0;
>         for (i = 0; i < 16; i++) {
> --
> 2.2.0.GIT

v2 patches for fixes on RHEL3

[Reuben Hawkins]

These patches add a few autoconfig checks for nanosecond resolution fields in
struct stat, CLOCK_MONOTONIC, and HMAC_CTX_cleanup.  I'm fairly sure I've fixed
the concerns/doubts the first set of patched raised.

(I rarely use git-send-email so forgive me if I botch this)

[PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC

[Reuben Hawkins]

CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3
systems being used in production.  This change makes compiling git
less tedious on older platforms without CLOCK_MONOTONIC.
---
 Makefile         |  4 ++++
 config.mak.uname |  1 +
 configure.ac     | 22 ++++++++++++++++++++++
 trace.c          |  2 +-
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 7482a4d..af551a0 100644
--- a/Makefile
+++ b/Makefile
@@ -1382,6 +1382,10 @@ ifdef HAVE_CLOCK_GETTIME
 	EXTLIBS += -lrt
 endif
 
+ifdef HAVE_CLOCK_MONOTONIC
+	BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC
+endif
+
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK = NoThanks
 endif
diff --git a/config.mak.uname b/config.mak.uname
index a2f380f..926773e 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -35,6 +35,7 @@ ifeq ($(uname_S),Linux)
 	LIBC_CONTAINS_LIBINTL = YesPlease
 	HAVE_DEV_TTY = YesPlease
 	HAVE_CLOCK_GETTIME = YesPlease
+	HAVE_CLOCK_MONOTONIC = YesPlease
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
 	HAVE_ALLOCA_H = YesPlease
diff --git a/configure.ac b/configure.ac
index dcc4bf0..424dec5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -923,6 +923,28 @@ AC_CHECK_LIB([iconv], [locale_charset],
                      [CHARSET_LIB=-lcharset])])
 GIT_CONF_SUBST([CHARSET_LIB])
 #
+# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
+GIT_CHECK_FUNC(clock_gettime,
+[HAVE_CLOCK_GETTIME=YesPlease],
+[HAVE_CLOCK_GETTIME=])
+GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])
+
+AC_DEFUN([CLOCK_MONOTONIC_SRC], [
+AC_LANG_PROGRAM([[
+#include <time.h>
+clockid_t id = CLOCK_MONOTONIC;
+]], [])])
+
+#
+# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.
+AC_MSG_CHECKING([for CLOCK_MONOTONIC])
+AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
+	[AC_MSG_RESULT([yes])
+	HAVE_CLOCK_MONOTONIC=YesPlease],
+	[AC_MSG_RESULT([no])
+	HAVE_CLOCK_MONOTONIC=])
+GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])
+#
 # Define NO_SETITIMER if you don't have setitimer.
 GIT_CHECK_FUNC(setitimer,
 [NO_SETITIMER=],
diff --git a/trace.c b/trace.c
index 4778608..bfbd48f 100644
--- a/trace.c
+++ b/trace.c
@@ -324,7 +324,7 @@ int trace_want(struct trace_key *key)
 	return !!get_trace_fd(key);
 }
 
-#ifdef HAVE_CLOCK_GETTIME
+#if defined(HAVE_CLOCK_GETTIME) && defined(HAVE_CLOCK_MONOTONIC)
 
 static inline uint64_t highres_nanos(void)
 {
-- 
2.2.0.68.g8f72f0c.dirty

Re: [PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC

[Eric Sunshine]

On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:
> CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3
> systems being used in production.  This change makes compiling git
> less tedious on older platforms without CLOCK_MONOTONIC.

The second sentence is implied by the very presence of this patch,
thus adds no value. I, personally, would drop it.

Also, your sign-off is missing (as mentioned in my previous review[1]).

> ---
> diff --git a/Makefile b/Makefile
> index 7482a4d..af551a0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1382,6 +1382,10 @@ ifdef HAVE_CLOCK_GETTIME
>         EXTLIBS += -lrt
>  endif
>
> +ifdef HAVE_CLOCK_MONOTONIC
> +       BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC
> +endif

You need to document this new Makefile variable (HAVE_CLOCK_MONOTONIC)
at the top of Makefile (as mentioned in my previous review[1]), so
that people who build without running 'configure' will know that they
may need to tweak it.

>  ifeq ($(TCLTK_PATH),)
>  NO_TCLTK = NoThanks
>  endif
> diff --git a/configure.ac b/configure.ac
> index dcc4bf0..424dec5 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -923,6 +923,28 @@ AC_CHECK_LIB([iconv], [locale_charset],
>                       [CHARSET_LIB=-lcharset])])
>  GIT_CONF_SUBST([CHARSET_LIB])
>  #
> +# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
> +GIT_CHECK_FUNC(clock_gettime,
> +[HAVE_CLOCK_GETTIME=YesPlease],
> +[HAVE_CLOCK_GETTIME=])
> +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])

You could simplify the above four lines to this one-liner:

    GIT_CHECK_FUNC(clock_gettime,
        GIT_CONF_SUBST([HAVE_CLOCK_GETTIME], [YesPlease]))

> +
> +AC_DEFUN([CLOCK_MONOTONIC_SRC], [
> +AC_LANG_PROGRAM([[
> +#include <time.h>
> +clockid_t id = CLOCK_MONOTONIC;
> +]], [])])

No need to pass empty trailing arguments in m4. It's customary to drop
them altogether (since they are implied).

> +#
> +# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.
> +AC_MSG_CHECKING([for CLOCK_MONOTONIC])
> +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
> +       [AC_MSG_RESULT([yes])
> +       HAVE_CLOCK_MONOTONIC=YesPlease],
> +       [AC_MSG_RESULT([no])
> +       HAVE_CLOCK_MONOTONIC=])
> +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])

Ditto regarding simplification:

    AC_MSG_CHECKING([for CLOCK_MONOTONIC])
    AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
        [AC_MSG_RESULT([yes])
        GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC], [YesPlease])],
        [AC_MSG_RESULT([no])])

> +#
>  # Define NO_SETITIMER if you don't have setitimer.
>  GIT_CHECK_FUNC(setitimer,
>  [NO_SETITIMER=],

[1]: http://article.gmane.org/gmane.comp.version-control.git/261630

Re: [PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC

[Reuben Hawkins]

On Wed, Jan 7, 2015 at 1:37 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:
>> CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3
>> systems being used in production.  This change makes compiling git
>> less tedious on older platforms without CLOCK_MONOTONIC.
>
> The second sentence is implied by the very presence of this patch,
> thus adds no value. I, personally, would drop it.
>
> Also, your sign-off is missing (as mentioned in my previous review[1]).
>
>> ---
>> diff --git a/Makefile b/Makefile
>> index 7482a4d..af551a0 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1382,6 +1382,10 @@ ifdef HAVE_CLOCK_GETTIME
>>         EXTLIBS += -lrt
>>  endif
>>
>> +ifdef HAVE_CLOCK_MONOTONIC
>> +       BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC
>> +endif
>
> You need to document this new Makefile variable (HAVE_CLOCK_MONOTONIC)
> at the top of Makefile (as mentioned in my previous review[1]), so
> that people who build without running 'configure' will know that they
> may need to tweak it.
>
>>  ifeq ($(TCLTK_PATH),)
>>  NO_TCLTK = NoThanks
>>  endif
>> diff --git a/configure.ac b/configure.ac
>> index dcc4bf0..424dec5 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -923,6 +923,28 @@ AC_CHECK_LIB([iconv], [locale_charset],
>>                       [CHARSET_LIB=-lcharset])])
>>  GIT_CONF_SUBST([CHARSET_LIB])
>>  #
>> +# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
>> +GIT_CHECK_FUNC(clock_gettime,
>> +[HAVE_CLOCK_GETTIME=YesPlease],
>> +[HAVE_CLOCK_GETTIME=])
>> +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])
>
> You could simplify the above four lines to this one-liner:
>
>     GIT_CHECK_FUNC(clock_gettime,
>         GIT_CONF_SUBST([HAVE_CLOCK_GETTIME], [YesPlease]))
>
>> +
>> +AC_DEFUN([CLOCK_MONOTONIC_SRC], [
>> +AC_LANG_PROGRAM([[
>> +#include <time.h>
>> +clockid_t id = CLOCK_MONOTONIC;
>> +]], [])])
>
> No need to pass empty trailing arguments in m4. It's customary to drop
> them altogether (since they are implied).
>
>> +#
>> +# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.
>> +AC_MSG_CHECKING([for CLOCK_MONOTONIC])
>> +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
>> +       [AC_MSG_RESULT([yes])
>> +       HAVE_CLOCK_MONOTONIC=YesPlease],
>> +       [AC_MSG_RESULT([no])
>> +       HAVE_CLOCK_MONOTONIC=])
>> +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])
>
> Ditto regarding simplification:
>
>     AC_MSG_CHECKING([for CLOCK_MONOTONIC])
>     AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
>         [AC_MSG_RESULT([yes])
>         GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC], [YesPlease])],
>         [AC_MSG_RESULT([no])])

I *think* there's an issue with this simplification as used right
here.  In the 'no' case, HAVE_CLOCK_MONOTONIC *must* be undefined by
setting it equal to nothing

HAVE_CLOCK_MONOTONIC=

So that the setting in config.mak.uname 'HAVE_CLOCK_MONOTINIC =
YesPlease' will be overridden.

So this one needs to stay as is.

>
>> +#
>>  # Define NO_SETITIMER if you don't have setitimer.
>>  GIT_CHECK_FUNC(setitimer,
>>  [NO_SETITIMER=],
>
> [1]: http://article.gmane.org/gmane.comp.version-control.git/261630

Re: [PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC

[Eric Sunshine]

On Wed, Jan 7, 2015 at 5:31 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:
> On Wed, Jan 7, 2015 at 1:37 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:
>>> +GIT_CHECK_FUNC(clock_gettime,
>>> +[HAVE_CLOCK_GETTIME=YesPlease],
>>> +[HAVE_CLOCK_GETTIME=])
>>> +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])
>>
>> You could simplify the above four lines to this one-liner:
>>
>>     GIT_CHECK_FUNC(clock_gettime,
>>         GIT_CONF_SUBST([HAVE_CLOCK_GETTIME], [YesPlease]))
>>
>>> +AC_MSG_CHECKING([for CLOCK_MONOTONIC])
>>> +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
>>> +       [AC_MSG_RESULT([yes])
>>> +       HAVE_CLOCK_MONOTONIC=YesPlease],
>>> +       [AC_MSG_RESULT([no])
>>> +       HAVE_CLOCK_MONOTONIC=])
>>> +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])
>>
>> Ditto regarding simplification:
>>
>>     AC_MSG_CHECKING([for CLOCK_MONOTONIC])
>>     AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
>>         [AC_MSG_RESULT([yes])
>>         GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC], [YesPlease])],
>>         [AC_MSG_RESULT([no])])
>
> I *think* there's an issue with this simplification as used right
> here.  In the 'no' case, HAVE_CLOCK_MONOTONIC *must* be undefined by
> setting it equal to nothing
>
> HAVE_CLOCK_MONOTONIC=
>
> So that the setting in config.mak.uname 'HAVE_CLOCK_MONOTINIC =
> YesPlease' will be overridden.
>
> So this one needs to stay as is.

Yes, you're right. That means that the HAVE_CLOCK_GETTIME
simplification also suffers the same shortcoming. So, neither
simplification is appropriate in this instance. Sorry for the noise.