[PATCH v7 1/3] compat: add a mkstemps() compatibility function

[PATCH v7 1/3] compat: add a mkstemps() compatibility function

[David Aguilar]

mkstemps() is a BSD extension so provide an implementation
for cross-platform use.

Signed-off-by: David Aguilar <davvid@gmail.com>
Tested-by: Johannes Sixt <j6t@kdbg.org> (Windows)
---
 Makefile          |   19 ++++++++++++++++
 compat/mkstemps.c |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 config.mak.in     |    1 +
 configure.ac      |    7 ++++++
 git-compat-util.h |    9 +++++++
 5 files changed, 99 insertions(+), 0 deletions(-)
 create mode 100644 compat/mkstemps.c

diff --git a/Makefile b/Makefile
index eaae45d..a70b5f0 100644
--- a/Makefile
+++ b/Makefile
@@ -52,6 +52,8 @@ all::
 #
 # Define NO_MKDTEMP if you don't have mkdtemp in the C library.
 #
+# Define NO_MKSTEMPS if you don't have mkstemps in the C library.
+#
 # Define NO_SYS_SELECT_H if you don't have sys/select.h.
 #
 # Define NO_SYMLINK_HEAD if you never want .git/HEAD to be a symbolic link.
@@ -636,10 +638,12 @@ EXTLIBS =
 
 ifeq ($(uname_S),Linux)
 	NO_STRLCPY = YesPlease
+	NO_MKSTEMPS = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
 	NO_STRLCPY = YesPlease
+	NO_MKSTEMPS = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
 endif
 ifeq ($(uname_S),UnixWare)
@@ -651,6 +655,7 @@ ifeq ($(uname_S),UnixWare)
 	SHELL_PATH = /usr/local/bin/bash
 	NO_IPV6 = YesPlease
 	NO_HSTRERROR = YesPlease
+	NO_MKSTEMPS = YesPlease
 	BASIC_CFLAGS += -Kthread
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
@@ -674,6 +679,7 @@ ifeq ($(uname_S),SCO_SV)
 	SHELL_PATH = /usr/bin/bash
 	NO_IPV6 = YesPlease
 	NO_HSTRERROR = YesPlease
+	NO_MKSTEMPS = YesPlease
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
 	NO_STRCASESTR = YesPlease
@@ -702,6 +708,7 @@ ifeq ($(uname_S),SunOS)
 	NO_MEMMEM = YesPlease
 	NO_HSTRERROR = YesPlease
 	NO_MKDTEMP = YesPlease
+	NO_MKSTEMPS = YesPlease
 	OLD_ICONV = UnfortunatelyYes
 	ifeq ($(uname_R),5.8)
 		NO_UNSETENV = YesPlease
@@ -724,6 +731,7 @@ ifeq ($(uname_O),Cygwin)
 	NO_D_INO_IN_DIRENT = YesPlease
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
+	NO_MKSTEMPS = YesPlease
 	NO_SYMLINK_HEAD = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
@@ -767,11 +775,13 @@ ifeq ($(uname_S),NetBSD)
 	BASIC_LDFLAGS += -L/usr/pkg/lib $(CC_LD_DYNPATH)/usr/pkg/lib
 	THREADED_DELTA_SEARCH = YesPlease
 	USE_ST_TIMESPEC = YesPlease
+	NO_MKSTEMPS = YesPlease
 endif
 ifeq ($(uname_S),AIX)
 	NO_STRCASESTR=YesPlease
 	NO_MEMMEM = YesPlease
 	NO_MKDTEMP = YesPlease
+	NO_MKSTEMPS = YesPlease
 	NO_STRLCPY = YesPlease
 	NO_NSEC = YesPlease
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
@@ -787,12 +797,14 @@ endif
 ifeq ($(uname_S),GNU)
 	# GNU/Hurd
 	NO_STRLCPY=YesPlease
+	NO_MKSTEMPS = YesPlease
 endif
 ifeq ($(uname_S),IRIX64)
 	NO_IPV6=YesPlease
 	NO_SETENV=YesPlease
 	NO_STRCASESTR=YesPlease
 	NO_MEMMEM = YesPlease
+	NO_MKSTEMPS = YesPlease
 	NO_STRLCPY = YesPlease
 	NO_SOCKADDR_STORAGE=YesPlease
 	SHELL_PATH=/usr/gnu/bin/bash
@@ -805,6 +817,7 @@ ifeq ($(uname_S),HP-UX)
 	NO_SETENV=YesPlease
 	NO_STRCASESTR=YesPlease
 	NO_MEMMEM = YesPlease
+	NO_MKSTEMPS = YesPlease
 	NO_STRLCPY = YesPlease
 	NO_MKDTEMP = YesPlease
 	NO_UNSETENV = YesPlease
@@ -834,6 +847,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	NO_C99_FORMAT = YesPlease
 	NO_STRTOUMAX = YesPlease
 	NO_MKDTEMP = YesPlease
+	NO_MKSTEMPS = YesPlease
 	SNPRINTF_RETURNS_BOGUS = YesPlease
 	NO_SVN_TESTS = YesPlease
 	NO_PERL_MAKEMAKER = YesPlease
@@ -853,6 +867,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 endif
 ifneq (,$(findstring arm,$(uname_M)))
 	ARM_SHA1 = YesPlease
+	NO_MKSTEMPS = YesPlease
 endif
 
 -include config.mak.autogen
@@ -1011,6 +1026,10 @@ ifdef NO_MKDTEMP
 	COMPAT_CFLAGS += -DNO_MKDTEMP
 	COMPAT_OBJS += compat/mkdtemp.o
 endif
+ifdef NO_MKSTEMPS
+	COMPAT_CFLAGS += -DNO_MKSTEMPS
+	COMPAT_OBJS += compat/mkstemps.o
+endif
 ifdef NO_UNSETENV
 	COMPAT_CFLAGS += -DNO_UNSETENV
 	COMPAT_OBJS += compat/unsetenv.o
diff --git a/compat/mkstemps.c b/compat/mkstemps.c
new file mode 100644
index 0000000..9ce729b
--- /dev/null
+++ b/compat/mkstemps.c
@@ -0,0 +1,63 @@
+#include "../git-compat-util.h"
+
+/* Adapted from libiberty's mkstemp.c. */
+int gitmkstemps(char *pattern, int suffix_len)
+{
+	static const char letters[] =
+		"abcdefghijklmnopqrstuvwxyz"
+		"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+		"0123456789";
+	static const int num_letters = 62;
+	uint64_t value;
+	struct timeval tv;
+	char *template;
+	size_t len;
+	int fd, count;
+
+	len = strlen(pattern);
+
+	if (len < 6 + suffix_len) {
+		errno = EINVAL;
+		return -1;
+	}
+
+	if (strncmp(&pattern[len - 6 - suffix_len], "XXXXXX", 6)) {
+		errno = EINVAL;
+		return -1;
+	}
+
+	/* Replace pattern's XXXXXX characters with randomness.
+	 * Try TMP_MAX different filenames.
+	 */
+	gettimeofday(&tv, NULL);
+	value = ((size_t)(tv.tv_usec << 16)) ^ tv.tv_sec ^ getpid();
+	template = &pattern[len - 6 - suffix_len];
+	for (count = 0; count < TMP_MAX; ++count) {
+		uint64_t v = value;
+		/* Fill in the random bits. */
+		template[0] = letters[v % num_letters]; v/= num_letters;
+		template[1] = letters[v % num_letters]; v/= num_letters;
+		template[2] = letters[v % num_letters]; v/= num_letters;
+		template[3] = letters[v % num_letters]; v/= num_letters;
+		template[4] = letters[v % num_letters]; v/= num_letters;
+		template[5] = letters[v % num_letters]; v/= num_letters;
+
+		fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, 0600);
+		if (fd > 0)
+			return fd;
+		/* Fatal error (EPERM, ENOSPC etc).
+		 * It doesn't make sense to loop.
+		 */
+		if (errno != EEXIST)
+			break;
+		/* This is a random value.  It is only necessary that
+		 * the next TMP_MAX values generated by adding 7777 to
+		 * VALUE are different with (module 2^32).
+		 */
+		value += 7777;
+	}
+	/* We return the null string if we can't find a unique file name.  */
+	pattern[0] = '\0';
+	errno = EINVAL;
+	return -1;
+}
diff --git a/config.mak.in b/config.mak.in
index 7cce0c1..b6619af 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -46,6 +46,7 @@ NO_STRTOUMAX=@NO_STRTOUMAX@
 NO_SETENV=@NO_SETENV@
 NO_UNSETENV=@NO_UNSETENV@
 NO_MKDTEMP=@NO_MKDTEMP@
+NO_MKSTEMPS=@NO_MKSTEMPS@
 NO_ICONV=@NO_ICONV@
 OLD_ICONV=@OLD_ICONV@
 NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
diff --git a/configure.ac b/configure.ac
index 4e728bc..953da07 100644
--- a/configure.ac
+++ b/configure.ac
@@ -677,6 +677,13 @@ GIT_CHECK_FUNC(mkdtemp,
 [NO_MKDTEMP=YesPlease])
 AC_SUBST(NO_MKDTEMP)
 #
+# Define NO_MKSTEMPS if you don't have mkstemps in the C library.
+GIT_CHECK_FUNC(mkstemps,
+[NO_MKSTEMPS=],
+[NO_MKSTEMPS=YesPlease])
+AC_SUBST(NO_MKSTEMPS)
+#
+#
 # Define NO_MMAP if you want to avoid mmap.
 #
 # Define NO_ICONV if your libc does not properly support iconv.
diff --git a/git-compat-util.h b/git-compat-util.h
index c7cf2d5..5a2d4e7 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -113,6 +113,10 @@
 #define PATH_MAX 4096
 #endif
 
+#ifndef TMP_MAX
+#define TMP_MAX 16384
+#endif
+
 #ifndef PRIuMAX
 #define PRIuMAX "llu"
 #endif
@@ -232,6 +236,11 @@ extern int gitsetenv(const char *, const char *, int);
 extern char *gitmkdtemp(char *);
 #endif
 
+#ifdef NO_MKSTEMPS
+#define mkstemps gitmkstemps
+extern int gitmkstemps(char *, int);
+#endif
+
 #ifdef NO_UNSETENV
 #define unsetenv gitunsetenv
 extern void gitunsetenv(const char *);
-- 
1.6.1.3

[PATCH v7 2/3] compat: add a basename() compatibility function

[David Aguilar]

Some systems such as Windows lack libgen.h so provide a
basename() implementation for cross-platform use.

This introduces the NO_LIBGEN_H construct to the Makefile
and autoconf scripts.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 Makefile          |    8 ++++++++
 compat/basename.c |   15 +++++++++++++++
 config.mak.in     |    1 +
 configure.ac      |    6 ++++++
 git-compat-util.h |    7 +++++++
 5 files changed, 37 insertions(+), 0 deletions(-)
 create mode 100644 compat/basename.c

diff --git a/Makefile b/Makefile
index a70b5f0..a59f106 100644
--- a/Makefile
+++ b/Makefile
@@ -54,6 +54,8 @@ all::
 #
 # Define NO_MKSTEMPS if you don't have mkstemps in the C library.
 #
+# Define NO_LIBGEN_H if you don't have libgen.h.
+#
 # Define NO_SYS_SELECT_H if you don't have sys/select.h.
 #
 # Define NO_SYMLINK_HEAD if you never want .git/HEAD to be a symbolic link.
@@ -834,6 +836,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	NO_PREAD = YesPlease
 	NO_OPENSSL = YesPlease
 	NO_CURL = YesPlease
+	NO_LIBGEN_H = YesPlease
 	NO_SYMLINK_HEAD = YesPlease
 	NO_IPV6 = YesPlease
 	NO_SETENV = YesPlease
@@ -899,6 +902,11 @@ ifndef CC_LD_DYNPATH
 	endif
 endif
 
+ifdef NO_LIBGEN_H
+	COMPAT_CFLAGS += -DNO_LIBGEN_H
+	COMPAT_OBJS += compat/basename.o
+endif
+
 ifdef NO_CURL
 	BASIC_CFLAGS += -DNO_CURL
 else
diff --git a/compat/basename.c b/compat/basename.c
new file mode 100644
index 0000000..d8f8a3c
--- /dev/null
+++ b/compat/basename.c
@@ -0,0 +1,15 @@
+#include "../git-compat-util.h"
+
+/* Adapted from libiberty's basename.c.  */
+char *gitbasename (char *path)
+{
+	const char *base;
+	/* Skip over the disk name in MSDOS pathnames. */
+	if (has_dos_drive_prefix(path))
+		path += 2;
+	for (base = path; *path; path++) {
+		if (is_dir_sep(*path))
+			base = path + 1;
+	}
+	return (char *)base;
+}
diff --git a/config.mak.in b/config.mak.in
index b6619af..e8d96e8 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -30,6 +30,7 @@ NEEDS_SSL_WITH_CRYPTO=@NEEDS_SSL_WITH_CRYPTO@
 NO_OPENSSL=@NO_OPENSSL@
 NO_CURL=@NO_CURL@
 NO_EXPAT=@NO_EXPAT@
+NO_LIBGEN_H=@NO_LIBGEN_H@
 NEEDS_LIBICONV=@NEEDS_LIBICONV@
 NEEDS_SOCKET=@NEEDS_SOCKET@
 NO_SYS_SELECT_H=@NO_SYS_SELECT_H@
diff --git a/configure.ac b/configure.ac
index 953da07..108a97f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -627,6 +627,12 @@ AC_SUBST(SNPRINTF_RETURNS_BOGUS)
 ## (in default C library and libraries checked by AC_CHECK_LIB)
 AC_MSG_NOTICE([CHECKS for library functions])
 #
+# Define NO_LIBGEN_H if you don't have libgen.h.
+AC_CHECK_HEADER([libgen.h],
+[NO_LIBGEN_H=],
+[NO_LIBGEN_H=YesPlease])
+AC_SUBST(NO_LIBGEN_H)
+#
 # Define NO_STRCASESTR if you don't have strcasestr.
 GIT_CHECK_FUNC(strcasestr,
 [NO_STRCASESTR=],
diff --git a/git-compat-util.h b/git-compat-util.h
index 5a2d4e7..d248047 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -97,6 +97,13 @@
 #include "compat/mingw.h"
 #endif	/* __MINGW32__ */
 
+#ifndef NO_LIBGEN_H
+#include <libgen.h>
+#else
+#define basename gitbasename
+extern char *gitbasename(char *);
+#endif
+
 #ifndef NO_ICONV
 #include <iconv.h>
 #endif
-- 
1.6.1.3

[PATCH v7 3/3] diff: generate pretty filenames in prep_temp_blob()

[David Aguilar]

Naturally, prep_temp_blob() did not care about filenames.
As a result, GIT_EXTERNAL_DIFF and textconv generated
filenames such as ".diff_XXXXXX".

This modifies prep_temp_blob() to generate user-friendly
filenames when creating temporary files.

Diffing "name.ext" now generates "XXXXXX_name.ext".

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 cache.h                  |    2 ++
 diff.c                   |   12 +++++++++++-
 path.c                   |   16 ++++++++++++++++
 t/t4020-diff-external.sh |    9 +++++++++
 4 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/cache.h b/cache.h
index b8503ad..871c984 100644
--- a/cache.h
+++ b/cache.h
@@ -614,6 +614,8 @@ extern int is_empty_blob_sha1(const unsigned char *sha1);
 
 int git_mkstemp(char *path, size_t n, const char *template);
 
+int git_mkstemps(char *path, size_t n, const char *template, int suffix_len);
+
 /*
  * NOTE NOTE NOTE!!
  *
diff --git a/diff.c b/diff.c
index dcfbcb0..4d0a5b9 100644
--- a/diff.c
+++ b/diff.c
@@ -1964,8 +1964,16 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp,
 {
 	int fd;
 	struct strbuf buf = STRBUF_INIT;
+	struct strbuf template = STRBUF_INIT;
+	char *path_dup = xstrdup(path);
+	const char *base = basename(path_dup);
 
-	fd = git_mkstemp(temp->tmp_path, PATH_MAX, ".diff_XXXXXX");
+	/* Generate "XXXXXX_basename.ext" */
+	strbuf_addstr(&template, "XXXXXX_");
+	strbuf_addstr(&template, base);
+
+	fd = git_mkstemps(temp->tmp_path, PATH_MAX, template.buf,
+			strlen(base) + 1);
 	if (fd < 0)
 		die("unable to create temp-file: %s", strerror(errno));
 	if (convert_to_working_tree(path,
@@ -1981,6 +1989,8 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp,
 	temp->hex[40] = 0;
 	sprintf(temp->mode, "%06o", mode);
 	strbuf_release(&buf);
+	strbuf_release(&template);
+	free(path_dup);
 }
 
 static struct diff_tempfile *prepare_temp_file(const char *name,
diff --git a/path.c b/path.c
index 8a0a674..047fdb0 100644
--- a/path.c
+++ b/path.c
@@ -139,6 +139,22 @@ int git_mkstemp(char *path, size_t len, const char *template)
 	return mkstemp(path);
 }
 
+/* git_mkstemps() - create tmp file with suffix honoring TMPDIR variable. */
+int git_mkstemps(char *path, size_t len, const char *template, int suffix_len)
+{
+	const char *tmp;
+	size_t n;
+
+	tmp = getenv("TMPDIR");
+	if (!tmp)
+		tmp = "/tmp";
+	n = snprintf(path, len, "%s/%s", tmp, template);
+	if (len <= n) {
+		errno = ENAMETOOLONG;
+		return -1;
+	}
+	return mkstemps(path, suffix_len);
+}
 
 int validate_headref(const char *path)
 {
diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index 0720001..4ea42e0 100755
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -136,6 +136,15 @@ test_expect_success 'GIT_EXTERNAL_DIFF with more than one changed files' '
 	GIT_EXTERNAL_DIFF=echo git diff
 '
 
+test_expect_success 'GIT_EXTERNAL_DIFF generates pretty paths' '
+	touch file.ext &&
+	git add file.ext &&
+	echo with extension > file.ext &&
+	GIT_EXTERNAL_DIFF=echo git diff file.ext | grep ......_file\.ext &&
+	git update-index --force-remove file.ext &&
+	rm file.ext
+'
+
 echo "#!$SHELL_PATH" >fake-diff.sh
 cat >> fake-diff.sh <<\EOF
 cat $2 >> crlfed.txt
-- 
1.6.1.3

Re: [PATCH v7 1/3] compat: add a mkstemps() compatibility function

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> mkstemps() is a BSD extension so provide an implementation
> for cross-platform use.
> ...
> diff --git a/git-compat-util.h b/git-compat-util.h
> index c7cf2d5..5a2d4e7 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -113,6 +113,10 @@
>  #define PATH_MAX 4096
>  #endif
>  
> +#ifndef TMP_MAX
> +#define TMP_MAX 16384
> +#endif

Giving fallback definition to PATH_MAX and PRIuMAX (as seen in the context
of the hunk) makes perfect sense.  TMP_MAX on the other hand is being
obsoleted in POSIX.1, and more importantly, we do not use it in our
program anywhere other than the compatibility implementation of
mkstemps().

I'll move this to the file that is used, which is compat/mkstemps.c.  I'll
further change the constant not to inherit from whatever the platform
happens to define TMP_MAX to be.  POSIX.1 says "at least 25", but the
fallback implementation does not need to be constrained to such a low
number.

Thanks.

Re: [PATCH v7 3/3] diff: generate pretty filenames in prep_temp_blob()

[Johannes Sixt]

David Aguilar schrieb:
> Naturally, prep_temp_blob() did not care about filenames.
> As a result, GIT_EXTERNAL_DIFF and textconv generated
> filenames such as ".diff_XXXXXX".
> 
> This modifies prep_temp_blob() to generate user-friendly
> filenames when creating temporary files.
> 
> Diffing "name.ext" now generates "XXXXXX_name.ext".
> 
> Signed-off-by: David Aguilar <davvid@gmail.com>

This series (as per 003b33a from Junio's pu):

Tested-by: Johannes Sixt <j6t@kdbg.org> (Windows)

-- Hannes