Re: [PATCH] log which temporary file could not be created

From

Arnout Engelen <arnouten@bzzt.net>

Date

Oct 18, 2010, 09:20 UTC

Message-ID

<20101018092000.GH9348@bzzt.net>

In-Reply-To

<7v8w24oygk.fsf@alter.siamese.dyndns.org>

On Mon, Oct 11, 2010 at 08:56:59PM -0700, Junio C Hamano wrote:

> Jonathan Nieder <jrnieder@gmail.com> writes:
> >> On Sat, Oct 09, 2010 at 09:41:24PM -0500, Jonathan Nieder wrote:
> >>> 	fatal: Unable to create temporary file '.merge_file_Sc7R5c': File exists
> >>> 	fatal: Unable to create temporary file 'newrepo/.git/tOWHcxk': No space left on device
> >
> > Converting the filename to an absolute path with make_absolute_path
> > might be useful, but I am not entirely sure it is worth the
> > complication.
> 
> I am not sure if it is worth the strdup "just in case it fails" either.

Below is an updated patch that:
- copies the original template to the stack to be able to log it in case 
  mkstemp clears it
- on failure, logs the modified template, if any, or the original one
- logs the CWD when the template is relative
- adds a test testing this gives sane output (and doesn't crash, etc)

Signed-off-by: Arnout Engelen <arnouten@bzzt.net>

---
 Makefile          |    1 +
 t/t0007-mktemp.sh |   16 ++++++++++++++++
 test-mktemp.c     |   25 +++++++++++++++++++++++++
 wrapper.c         |   30 ++++++++++++++++++++++++++----
 4 files changed, 68 insertions(+), 4 deletions(-)
 create mode 100755 t/t0007-mktemp.sh
 create mode 100644 test-mktemp.c

diff --git a/Makefile b/Makefile
index 1f1ce04..30aafa2 100644
--- a/Makefile
+++ b/Makefile
@@ -428,6 +428,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/t0007-mktemp.sh b/t/t0007-mktemp.sh
new file mode 100755
index 0000000..7abeeca
--- /dev/null
+++ b/t/t0007-mktemp.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+test_description='test creating temporary files'
+. ./test-lib.sh
+
+check_errormsg() {
+	test_expect_success "$1" "
+		test-mktemp $2 &>actual
+		grep \"$3\" actual
+		"
+}
+
+check_errormsg "Creating temporary file in nonexisting directory" n "Unable to create temporary file '/tmp/does/not/exist/test"
+check_errormsg "Creating temporary file in invalid relative directory" p "at /"
+
+test_done
diff --git a/test-mktemp.c b/test-mktemp.c
new file mode 100644
index 0000000..fad2908
--- /dev/null
+++ b/test-mktemp.c
@@ -0,0 +1,25 @@
+/*
+ * test-mktemp.c: code to exercise the creation of temporary files
+ */
+#include "wrapper.h"
+
+int main(int argc, char *argv[])
+{
+	if (argc != 2) {
+		fprintf(stderr, "Expected 1 parameter defining the situation to test");
+		exit(-1);
+	}
+	switch (argv[1][0]) {
+		case 'n':
+			// temporary file in nonexistent directory
+			xmkstemp(strdup("/tmp/does/not/exist/testXXXXXX"));
+			break;
+		case 'p':
+			// temporary file in directory where we have no write permissions
+			xmkstemp(strdup("../../foo/testXXXXXX"));
+			break;
+		default:
+			fprintf(stderr, "No such test case: %s\n", argv[0]);
+	}
+	return 0;
+}
diff --git a/wrapper.c b/wrapper.c
index fd8ead3..b588e67 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -212,20 +212,42 @@ FILE *xfdopen(int fd, const char *mode)
 int xmkstemp(char *template)
 {
 	int fd;
+	char originalTemplate[255];
+	strncpy(originalTemplate, template, 255);
+	originalTemplate[254] = '\0';
 
 	fd = mkstemp(template);
-	if (fd < 0)
-		die_errno("Unable to create temporary file");
+	if (fd < 0) {
+		if (strlen(template) == 0) {
+			template = originalTemplate;
+		}
+		if (*template == '/') {
+			die_errno("Unable to create temporary file '%s'", template);
+		} else {
+			die_errno("Unable to create temporary file '%s' at %s", template, getcwd(NULL, 0));
+		}
+	}
 	return fd;
 }
 
 int xmkstemp_mode(char *template, int mode)
 {
 	int fd;
+	char originalTemplate[255];
+	strncpy(originalTemplate, template, 255);
+	originalTemplate[254] = '\0';
 
 	fd = git_mkstemp_mode(template, mode);
-	if (fd < 0)
-		die_errno("Unable to create temporary file");
+	if (fd < 0) {
+		if (strlen(template) == 0) {
+			template = originalTemplate;
+		}
+		if (*template == '/') {
+			die_errno("Unable to create temporary file '%s'", template);
+		} else {
+			die_errno("Unable to create temporary file '%s' at %s", template, getcwd(NULL, 0));
+		}
+	}
 	return fd;
 }
 

-- 
1.7.2.3