From: Joel Rosdahl <joel@rosdahl.net>
Date: Sat, 15 Nov 2014 12:16:40 +0000 (+0100)
Subject: Improve temporary file creation functions and usage
X-Git-Tag: v3.2~8
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fc61ca9a9232f26ee3f714d4b8738d916f6948ff;p=thirdparty%2Fccache.git

Improve temporary file creation functions and usage
---

diff --git a/ccache.c b/ccache.c
index dcdb3f430..0f5ca1f1a 100644
--- a/ccache.c
+++ b/ccache.c
@@ -736,14 +736,10 @@ to_cache(struct args *args)
 	struct stat st;
 	int status;
 
-	if (create_parent_dirs(cached_obj) != 0) {
-		fatal("Failed to create parent directory for %s: %s",
-		      cached_obj, strerror(errno));
-	}
-	tmp_stdout = format("%s.tmp.stdout.%s", cached_obj, tmp_string());
-	create_empty_file(tmp_stdout);
-	tmp_stderr = format("%s.tmp.stderr.%s", cached_obj, tmp_string());
-	create_empty_file(tmp_stderr);
+	tmp_stdout = format("%s.tmp.stdout", cached_obj);
+	create_empty_tmp_file(&tmp_stdout);
+	tmp_stderr = format("%s.tmp.stderr", cached_obj);
+	create_empty_tmp_file(&tmp_stderr);
 
 	args_add(args, "-o");
 	args_add(args, output_obj);
@@ -974,23 +970,18 @@ get_object_name_from_cpp(struct args *args, struct mdfour *hash)
 		input_base[10] = 0;
 	}
 
-	path_stderr = format("%s/tmp.cpp_stderr.%s", temp_dir(), tmp_string());
-	if (create_parent_dirs(path_stderr) != 0) {
-		fatal("Failed to create parent directory for %s: %s\n",
-		      path_stderr, strerror(errno));
-	}
-	create_empty_file(path_stderr);
+	path_stderr = format("%s/tmp.cpp_stderr", temp_dir());
+	create_empty_tmp_file(&path_stderr);
 	add_pending_tmp_file(path_stderr);
 
 	time_of_compilation = time(NULL);
 
 	if (!direct_i_file) {
 		/* The temporary file needs the proper i_extension for the compiler to do
-		 * its thing. However, create_empty_file requires the tmp_string() part to
-		 * be last, which is why the temporary file is created in two steps. */
-		char *path_stdout_tmp =
-			format("%s/%s.tmp.%s", temp_dir(), input_base, tmp_string());
-		create_empty_file(path_stdout_tmp);
+		 * its thing. However, create_empty_tmp_file appends a suffix to the
+		 * filename, which is why the temporary file is created in two steps. */
+		char *path_stdout_tmp = format("%s/%s.tmp", temp_dir(), input_base);
+		create_empty_tmp_file(&path_stdout_tmp);
 		path_stdout = format("%s.%s", path_stdout_tmp, conf->cpp_extension);
 		x_rename(path_stdout_tmp, path_stdout);
 		free(path_stdout_tmp);
diff --git a/ccache.h b/ccache.h
index a98bd94e2..241f63194 100644
--- a/ccache.h
+++ b/ccache.h
@@ -150,7 +150,9 @@ char *gnu_getcwd(void);
 #ifndef HAVE_STRTOK_R
 char *strtok_r(char *str, const char *delim, char **saveptr);
 #endif
-int create_empty_file(char *fname);
+int create_tmp_fd(char **fname);
+FILE *create_tmp_file(char **fname, const char *mode);
+void create_empty_tmp_file(char **fname);
 const char *get_home_directory(void);
 char *get_cwd(void);
 bool same_executable_name(const char *s1, const char *s2);
diff --git a/conf.c b/conf.c
index 56adb484e..92c70c064 100644
--- a/conf.c
+++ b/conf.c
@@ -474,8 +474,8 @@ conf_set_value_in_file(const char *path, const char *key, const char *value,
 		return false;
 	}
 
-	outpath = format("%s.tmp.%s", path, tmp_string());
-	outfile = fopen(outpath, "w");
+	outpath = format("%s.tmp", path);
+	outfile = create_tmp_file(&outpath, "w");
 	if (!outfile) {
 		*errmsg = format("%s: %s", outpath, strerror(errno));
 		free(outpath);
diff --git a/manifest.c b/manifest.c
index 4d652eedd..6eb53fd92 100644
--- a/manifest.c
+++ b/manifest.c
@@ -736,18 +736,8 @@ manifest_put(const char *manifest_path, struct file_hash *object_hash,
 		mf = create_empty_manifest();
 	}
 
-	tmp_file = format("%s.tmp.%s", manifest_path, tmp_string());
-	fd2 = mkstemp(tmp_file);
-	if (fd2 == -1 && errno == ENOENT) {
-		if (create_parent_dirs(tmp_file) == 0) {
-			reformat(&tmp_file, "%s.tmp.%s", manifest_path, tmp_string());
-			fd2 = mkstemp(tmp_file);
-		}
-	}
-	if (fd2 == -1) {
-		cc_log("Failed to open %s", tmp_file);
-		goto out;
-	}
+	tmp_file = format("%s.tmp", manifest_path);
+	fd2 = create_tmp_fd(&tmp_file);
 	f2 = gzdopen(fd2, "wb");
 	if (!f2) {
 		cc_log("Failed to gzdopen %s", tmp_file);
diff --git a/stats.c b/stats.c
index 16aaa9fe3..71fbb7af6 100644
--- a/stats.c
+++ b/stats.c
@@ -130,35 +130,16 @@ stats_write(const char *path, struct counters *counters)
 	size_t i;
 	char *tmp_file;
 	FILE *f;
-	int fd;
-
-	tmp_file = format("%s.tmp.%s", path, tmp_string());
-	fd = mkstemp(tmp_file);
-	if (fd == -1 && errno == ENOENT) {
-		if (create_parent_dirs(tmp_file) == 0) {
-			reformat(&tmp_file, "%s.tmp.%s", path, tmp_string());
-			fd = mkstemp(tmp_file);
-		}
-	}
-	if (fd == -1) {
-		cc_log("Failed to open %s", tmp_file);
-		close(fd);
-		goto end;
-	}
-	f = fdopen(fd, "wb");
-	if (!f) {
-		cc_log("Failed to open %s", tmp_file);
-		goto end;
-	}
+
+	tmp_file = format("%s.tmp", path);
+	f = create_tmp_file(&tmp_file, "wb");
 	for (i = 0; i < counters->size; i++) {
 		if (fprintf(f, "%u\n", counters->data[i]) < 0) {
 			fatal("Failed to write to %s", tmp_file);
 		}
 	}
-	fclose(f); /* This also implicitly closes the fd. */
+	fclose(f);
 	x_rename(tmp_file, path);
-
-end:
 	free(tmp_file);
 }
 
diff --git a/util.c b/util.c
index 8dbf01a45..072f53b9b 100644
--- a/util.c
+++ b/util.c
@@ -241,15 +241,9 @@ copy_file(const char *src, const char *dest, int compress_level)
 	struct stat st;
 	int errnum;
 
-	tmp_name = format("%s.%s", dest, tmp_string());
-
 	/* open destination file */
-	fd_out = mkstemp(tmp_name);
-	if (fd_out == -1) {
-		cc_log("mkstemp error: %s", strerror(errno));
-		goto error;
-	}
-
+	tmp_name = x_strdup(dest);
+	fd_out = create_tmp_fd(&tmp_name);
 	cc_log("Copying %s to %s via %s (%scompressed)",
 	       src, dest, tmp_name, compress_level > 0 ? "" : "un");
 
@@ -1022,19 +1016,53 @@ strtok_r(char *str, const char *delim, char **saveptr)
 }
 #endif
 
-/* create an empty file */
+/*
+ * Create an empty temporary file. *fname will be reallocated and set to the
+ * resulting filename. Returns an open file descriptor to the file.
+ */
 int
-create_empty_file(char *fname)
+create_tmp_fd(char **fname)
 {
-	int fd;
-
-	fd = mkstemp(fname);
+	char *template = format("%s.%s", *fname, tmp_string());
+	int fd = mkstemp(template);
+	if (fd == -1 && errno == ENOENT) {
+		if (create_parent_dirs(template) != 0) {
+			fatal("Failed to create directory %s: %s",
+			      dirname(template), strerror(errno));
+		}
+		reformat(&template, "%s.%s", *fname, tmp_string());
+		fd = mkstemp(template);
+	}
 	if (fd == -1) {
-		cc_log("Failed to create %s: %s", fname, strerror(errno));
-		return -1;
+		fatal("Failed to create file %s: %s", template, strerror(errno));
 	}
-	close(fd);
-	return 0;
+	free(*fname);
+	*fname = template;
+	return fd;
+}
+
+/*
+ * Create an empty temporary file. *fname will be reallocated and set to the
+ * resulting filename. Returns an open FILE*.
+ */
+FILE *
+create_tmp_file(char **fname, const char *mode)
+{
+	FILE *file = fdopen(create_tmp_fd(fname), mode);
+	if (!file) {
+		fatal("Failed to create file %s: %s", *fname, strerror(errno));
+	}
+	return file;
+}
+
+/*
+ * Create an empty temporary file. *fname will be reallocated and set to the
+ * resulting filename.
+ */
+void
+create_empty_tmp_file(char **fname)
+{
+	close(create_tmp_fd(fname));
 }
 
 /*
@@ -1263,7 +1291,7 @@ x_unlink(const char *path)
 	 * file. We don't care if the temp file is trashed, so it's always safe to
 	 * unlink it first.
 	 */
-	char *tmp_name = format("%s.%s.rmXXXXXX", path, tmp_string());
+	char *tmp_name = format("%s.rm.%s", path, tmp_string());
 	int result = 0;
 	cc_log("Unlink %s via %s", path, tmp_name);
 	if (x_rename(path, tmp_name) == -1) {