[ccache] patch to avoid empty stderr cache files

Brian Foley bfoley at compsoc.nuigalway.ie
Mon Jun 30 22:12:51 EST 2003


On Sat, Jun 07, 2003 at 11:29:51AM +0200, Koblinger Egmont wrote:
> 
> Hi,
> 
> > I noticed, however, that it generates a .stderr file in the cache
> > directory for each compiled file, even if there is no output on stderr.
> > This is a little bit wasteful -- we don't need the stderr file in the
> > very common case of the compiler not producing any warnings.
> 
> Nice idea.
> 
> Just one comment. It seems to me that you place the output file in the
> cache first and stderr later. This way if ccache is interrupted between
> these two steps it will result in a syntactically valid but false cache
> content (recompiling the same code will swallow stderr). So the stderr
> file should be placed first in the cache and the output afterwards.

Good point. I tried to think of edge conditions that might cause
problems, but obviously I missed this one.

Fortunately the fix is very simple, and exactly as you suggest.

Thanks!
Brian.
-------------- next part --------------
--- ccache.c~	Sun May 25 10:29:41 2003
+++ ccache.c	Mon May 26 19:31:19 2003
@@ -143,7 +143,7 @@
 	char *path_stderr;
 	char *tmp_stdout, *tmp_stderr, *tmp_hashname;
 	struct stat st1, st2;
-	int status;
+	int status, fail;
 
 	x_asprintf(&tmp_stdout, "%s/tmp.stdout.%s", cache_dir, tmp_string());
 	x_asprintf(&tmp_stderr, "%s/tmp.stderr.%s", cache_dir, tmp_string());
@@ -209,17 +209,27 @@
 
 	x_asprintf(&path_stderr, "%s.stderr", hashname);
 
-	if (stat(tmp_stderr, &st1) != 0 ||
-	    stat(tmp_hashname, &st2) != 0 ||
-	    rename(tmp_hashname, hashname) != 0 ||
-	    rename(tmp_stderr, path_stderr) != 0) {
-		cc_log("failed to rename tmp files - %s\n", strerror(errno));
+
+        fail = (stat(tmp_stderr, &st1) != 0);
+        fail = fail || (stat(tmp_hashname, &st2) != 0);
+        if (!fail) {
+            if (st1.st_size == 0) {
+                /* If the stderr file is empty, delete it to save an inode */
+                unlink(tmp_stderr);
+            } else {
+                fail = fail || (rename(tmp_stderr, path_stderr) != 0);
+            }
+        }
+        fail = fail || (rename(tmp_hashname, hashname) != 0);
+        
+        if (fail) {
+		cc_log("failed to rename tmp files (%s %s) - %s\n", tmp_hashname, tmp_stderr, strerror(errno));
 		stats_update(STATS_ERROR);
 		failed();
 	}
 
 	cc_log("Placed %s into cache\n", output_file);
-	stats_tocache(file_size(&st1) + file_size(&st2));
+	stats_tocache(file_size(&st1) + file_size(&st2), (st1.st_size == 0));
 
 	free(tmp_hashname);
 	free(tmp_stderr);
@@ -391,31 +401,28 @@
 	int ret;
 	struct stat st;
 
-	x_asprintf(&stderr_file, "%s.stderr", hashname);
-	fd_stderr = open(stderr_file, O_RDONLY);
-	if (fd_stderr == -1) {
-		/* it isn't in cache ... */
-		free(stderr_file);
-		return;
-	}
-
-	/* make sure the output is there too */
+	/* make sure the output is in the cache */
 	if (stat(hashname, &st) != 0) {
-		close(fd_stderr);
-		unlink(stderr_file);
-		free(stderr_file);
 		return;
 	}
+        
+        /* check if stderr is too. If it isn't we
+           know that stderr must have been empty. */
+	x_asprintf(&stderr_file, "%s.stderr", hashname);
+	fd_stderr = open(stderr_file, O_RDONLY);
 
 	/* the user might be disabling cache hits */
 	if (first && getenv("CCACHE_RECACHE")) {
-		close(fd_stderr);
-		unlink(stderr_file);
+		if (fd_stderr != -1) {
+                    close(fd_stderr);
+                    unlink(stderr_file);
+                
+                }
 		free(stderr_file);
 		return;
 	}
 
-	utime(stderr_file, NULL);
+	if (fd_stderr != -1) utime(stderr_file, NULL);
 
 	if (strcmp(output_file, "/dev/null") == 0) {
 		ret = 0;
@@ -432,8 +439,11 @@
 	if (ret == -1 && errno == ENOENT) {
 		cc_log("hashfile missing for %s\n", output_file);
 		stats_update(STATS_MISSING);
-		close(fd_stderr);
-		unlink(stderr_file);
+		if (fd_stderr != -1) {
+                    close(fd_stderr);
+                    unlink(stderr_file);
+                }
+                free(stderr_file);
 		return;
 	}
 	free(stderr_file);
@@ -469,9 +479,11 @@
 		cpp_stderr = NULL;
 	}
 
-	/* send the stderr */
-	copy_fd(fd_stderr, 2);
-	close(fd_stderr);
+        /* send the stderr, if applicable */
+        if (fd_stderr != -1) {
+            copy_fd(fd_stderr, 2);
+            close(fd_stderr);
+        }
 
 	/* and exit with the right status code */
 	if (first) {
@@ -813,7 +825,6 @@
 /* the main program when not doing a compile */
 static int ccache_main(int argc, char *argv[])
 {
-	extern int optind;
 	int c;
 	size_t v;
 
--- ccache.h~	Mon May 26 19:17:36 2003
+++ ccache.h	Mon May 26 19:17:58 2003
@@ -94,7 +94,7 @@
 void stats_update(enum stats stat);
 void stats_zero(void);
 void stats_summary(void);
-void stats_tocache(size_t size);
+void stats_tocache(size_t size, int has_stderr);
 void stats_read(const char *stats_file, unsigned counters[STATS_END]);
 void stats_set_limits(long maxfiles, long maxsize);
 size_t value_units(const char *s);
--- stats.c~	Mon May 26 19:03:21 2003
+++ stats.c	Mon May 26 19:18:39 2003
@@ -116,7 +116,7 @@
 }
 
 /* update the stats counter for this compile */
-static void stats_update_size(enum stats stat, size_t size)
+static void stats_update_size(enum stats stat, size_t size, int has_stderr)
 {
 	int fd;
 	unsigned counters[STATS_END];
@@ -147,7 +147,11 @@
 
 	/* on a cache miss we up the file count and size */
 	if (stat == STATS_TOCACHE) {
-		counters[STATS_NUMFILES] += 2;
+		if (has_stderr) {
+                    counters[STATS_NUMFILES] += 1;
+                } else {
+                    counters[STATS_NUMFILES] += 2;
+                }
 		counters[STATS_TOTALSIZE] += size;
 	}
 
@@ -173,18 +177,18 @@
 }
 
 /* record a cache miss */
-void stats_tocache(size_t size)
+void stats_tocache(size_t size, int has_stderr)
 {
 	/* convert size to kilobytes */
 	size = size / 1024;
 
-	stats_update_size(STATS_TOCACHE, size);
+	stats_update_size(STATS_TOCACHE, size, has_stderr);
 }
 
 /* update a normal stat */
 void stats_update(enum stats stat)
 {
-	stats_update_size(stat, 0);
+	stats_update_size(stat, 0, 0);
 }
 
 /* read in the stats from one dir and add to the counters */


More information about the ccache mailing list