Re: [PATCH 1/2] read-cache: fix memleak

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> `ce` is allocated in make_cache_entry and should be freed if it is not
> used any more. refresh_cache_entry as a wrapper around refresh_cache_ent
> will either return `ce` or a new updated cache entry which is allocated
> to new memory. In that case we need to free `ce` ourselfs.

Thanks.  I started "commit --amend" to do s/ourselfs/ourselves/, but
ended up tweaking the description a bit more.

-- .8 --
From: Stefan Beller <sbeller@google.com>
Date: Mon, 23 Mar 2015 10:57:11 -0700
Subject: [PATCH] read-cache: fix memleak

`ce` is allocated in make_cache_entry and should be freed if it is not
used any more. refresh_cache_entry as a wrapper around refresh_cache_ent
will either return

 - the `ce` given as the parameter, when it was up-to-date;
 - a new updated cache entry which is allocated to new memory; or
 - a NULL when refreshing failed.

In the latter two cases, the original cache-entry `ce` is not used
and needs to be freed.  The rule can be expressed as "if the return
value from refresh is different from the original ce, ce is no
longer used."

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 read-cache.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 5b922fd5..0052b72 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -748,12 +748,9 @@ struct cache_entry *make_cache_entry(unsigned int mode,
 	ce->ce_mode = create_ce_mode(mode);
 
 	ret = refresh_cache_entry(ce, refresh_options);
-	if (!ret) {
+	if (ret != ce)
 		free(ce);
-		return NULL;
-	} else {
-		return ret;
-	}
+	return ret;
 }
 
 int ce_same_name(const struct cache_entry *a, const struct cache_entry *b)
-- 
2.3.3-454-g85aa98f