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

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Jul 14, 2010, 12:19 UTC
Message-ID: <20100714121925.GG15270@neumann>
In-Reply-To: <7v630k6wpg.fsf@alter.siamese.dyndns.org>

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

Show 10 quoted lines

> SZEDER Gábor <szeder@ira.uka.de> writes:
> > +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;