(unknown)
On Tue, Feb 17, 2015 at 10:14 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> Stefan Beller <sbeller@google.com> 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.
Show 7 quoted lines
>
> 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@google.com>
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@google.com>
---
read-cache.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
Show changes to read-cache.c +8 −2
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
Stefan Beller