Re: [PATCH 2/2] rerere: fix overeager gc

On Mon, Jul 12, 2010 at 05:40:11PM -0700, Junio C Hamano wrote:

SZEDER Gábor [off-list ref] writes:

quoted

+static time_t rerere_last_used_at(const char *name)
+{
+	struct stat st;
+	return stat(rerere_path(name, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
+}

Doesn't has_rerere_resolution() already do a stat on this path?  There are
only two allers of the function so it would probably make sense to pass a
pointer to struct stat from the caller to avoid one extra call to stat.

rerere_last_used_at() returns 0 when the stat() on 'postimage' fails,
exactly like has_rerere_resolution().  Consequently, we can use
rerere_last_used_at() to determine whether a resolution exists, too.
And if we do that, we dont have to "pollute" callers of
has_rerere_resolution() with superfluous struct stat variables.

So how about this squashed in instead?

-- >8 --

diff --git a/builtin/rerere.c b/builtin/rerere.c
index 52e4b64..1dc424b 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c

@@ -50,7 +50,7 @@ static void garbage_collect(struct string_list *rr)
 	DIR *dir;
 	struct dirent *e;
 	int i, cutoff;
-	time_t now = time(NULL), then;
+	time_t now = time(NULL), then, then_post;
 
 	git_config(git_rerere_gc_config, NULL);
 	dir = opendir(git_path("rr-cache"));

@@ -62,10 +62,9 @@ static void garbage_collect(struct string_list *rr)
 		then = rerere_created_at(e->d_name);
 		if (!then)
 			continue;
-		if (has_rerere_resolution(e->d_name)) {
-			then = rerere_last_used_at(e->d_name);
-			if (!then)
-				continue;
+		then_post = rerere_last_used_at(e->d_name);
+		if (then_post) {
+			then = then_post;
 			cutoff = cutoff_resolve;
 		} else
 			cutoff = cutoff_noresolve;