Re: [PATCH] Improved error messages when temporary file creation fails

[Arnout Engelen <arnouten@bzzt.net>]

Updated version of the patch, taking into account the feedback from this 
thread. (the use of a fixed-length buffer to temporarily store the template
for the error path is now sort of moot since make_nonrelative_path doesn't 
allow filenames larger than PATH_MAX anyway)

Signed-off-by: Arnout Engelen <arnouten@bzzt.net>
---
 Makefile               |    1 +
 t/t0070-fundamental.sh |   13 +++++++++++++
 test-mktemp.c          |   15 +++++++++++++++
 wrapper.c              |   32 ++++++++++++++++++++++++++++----
 4 files changed, 57 insertions(+), 4 deletions(-)
 create mode 100644 test-mktemp.c

diff --git a/Makefile b/Makefile
index 57d9c65..03a51cb 100644
--- a/Makefile
+++ b/Makefile
@@ -434,6 +434,7 @@ TEST_PROGRAMS_NEED_X += test-string-pool
 TEST_PROGRAMS_NEED_X += test-svn-fe
 TEST_PROGRAMS_NEED_X += test-treap
 TEST_PROGRAMS_NEED_X += test-index-version
+TEST_PROGRAMS_NEED_X += test-mktemp
 
 TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))
 
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 680d7d6..9bee8bf 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -12,4 +12,17 @@ test_expect_success 'character classes (isspace, isalpha etc.)' '
 	test-ctype
 '
 
+test_expect_success 'mktemp to nonexistent directory prints filename' '
+	test_must_fail test-mktemp doesnotexist/testXXXXXX 2>err &&
+	grep "doesnotexist/test" err
+'
+
+test_expect_success POSIXPERM 'mktemp to unwritable directory prints filename' '
+	mkdir cannotwrite &&
+	chmod -w cannotwrite &&
+	test_when_finished "chmod +w cannotwrite" &&
+	test_must_fail test-mktemp cannotwrite/testXXXXXX 2>err &&
+	grep "cannotwrite/test" err
+'
+
 test_done
diff --git a/test-mktemp.c b/test-mktemp.c
new file mode 100644
index 0000000..00fdd78
--- /dev/null
+++ b/test-mktemp.c
@@ -0,0 +1,15 @@
+/*
+ * test-mktemp.c: code to exercise the creation of temporary files
+ */
+#include <string.h>
+#include "git-compat-util.h"
+
+int main(int argc, char *argv[])
+{
+	if (argc != 2)
+		usage("Expected 1 parameter defining the temporary file template");
+
+	xmkstemp(strdup(argv[1]));
+
+	return 0;
+}
diff --git a/wrapper.c b/wrapper.c
index 4c1639f..9af40ee 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -196,10 +196,22 @@ FILE *xfdopen(int fd, const char *mode)
 int xmkstemp(char *template)
 {
 	int fd;
+	char origtemplate[PATH_MAX];
+	strlcpy(origtemplate, template, sizeof(origtemplate));
 
 	fd = mkstemp(template);
-	if (fd < 0)
-		die_errno("Unable to create temporary file");
+	if (fd < 0) {
+		int saved_errno = errno;
+		const char * nonrelative_template;
+
+		if (!template[0])
+			template = origtemplate;
+
+		nonrelative_template = make_nonrelative_path(template);
+		errno = saved_errno;
+		die_errno("Unable to create temporary file '%s'", 
+			nonrelative_template);
+	}
 	return fd;
 }
 
@@ -319,10 +331,22 @@ int gitmkstemps(char *pattern, int suffix_len)
 int xmkstemp_mode(char *template, int mode)
 {
 	int fd;
+	char origtemplate[PATH_MAX];
+	strlcpy(origtemplate, template, sizeof(origtemplate));
 
 	fd = git_mkstemp_mode(template, mode);
-	if (fd < 0)
-		die_errno("Unable to create temporary file");
+	if (fd < 0) {
+		int saved_errno = errno;
+		const char * nonrelative_template;
+
+		if (!template[0])
+			template = origtemplate;
+
+		nonrelative_template = make_nonrelative_path(template);
+		errno = saved_errno;
+		die_errno("Unable to create temporary file '%s'", 
+			nonrelative_template);
+	}
 	return fd;
 }
 
-- 
1.7.2.3