On Tue, Feb 17, 2015 at 10:14 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> This fixes a memory leak, when building the cache entries as
>> refresh_cache_entry may decide to return NULL in case of
>>
>
> in case of what?
Yeah, I got distracted when rewriting the commit message as I was
looking at refresh_cache_ent() wondering if there is a better place to
free the memory. Just as you said below.
Maybe we can drop that part of the sentence as it doesn't matter
why refresh_cache_ent() returns NULL. All that matters is the
possibility of it returning NULL.
>
> I briefly wondered if refresh_cache_ent() should do the freeing but
> that does not make sense at all if done unconditionally because the
> other caller of the function does want to retain ce on error, and it
> makes little sense to invent FREE_CE_ON_ERROR bit in refresh_option,
> either, so this fix looks sensible.
>
So here is a reworded commit message:
---8<---
>From 3a1f646c1dbe828b68e1b269290d2b5593f86455 Mon Sep 17 00:00:00 2001
From: Stefan Beller <sbeller@xxxxxxxxxx>
Date: Tue, 17 Feb 2015 10:05:36 -0800
Subject: [PATCH] read-cache.c: free cache entry when refreshing fails
This fixes a memory leak when building the cache entries as
refresh_cache_entry may decide to return NULL, but it doesn't
free the cache entry structure which was passed in as an argument.
I am not sure how severe this memory leak is as it was found by
scan.coverity.com, CID 290041.
Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
---
read-cache.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 9cff715..8d71860 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -725,7 +725,7 @@ struct cache_entry *make_cache_entry(unsigned int mode,
unsigned int refresh_options)
{
int size, len;
- struct cache_entry *ce;
+ struct cache_entry *ce, *ret;
if (!verify_path(path)) {
error("Invalid path '%s'", path);
@@ -742,7 +742,13 @@ struct cache_entry *make_cache_entry(unsigned int mode,
ce->ce_namelen = len;
ce->ce_mode = create_ce_mode(mode);
- return refresh_cache_entry(ce, refresh_options);
+ ret = refresh_cache_entry(ce, refresh_options);
+ if (!ret) {
+ free(ce);
+ return NULL;
+ } else {
+ return ret;
+ }
}
int ce_same_name(const struct cache_entry *a, const struct cache_entry *b)
--
2.3.0.81.gc37f363
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html