# [PATCH] builtin-update-index.c (add_file_to_cache): Don't leak a cache entry. 3 messages from 2007-02-16 to 2007-02-18. Participants: Jim Meyering, Linus Torvalds. Thread: https://gitlist.dev/t/6843 ## Jim Meyering, 2007-02-16 09:01 Subject: [PATCH] builtin-update-index.c (add_file_to_cache): Don't leak a cache entry. Message-ID: <87mz3e327y.fsf@rho.meyering.net> URL: https://gitlist.dev/e/87mz3e327y.fsf%40rho.meyering.net ``` Signed-off-by: Jim Meyering --- builtin-update-index.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/builtin-update-index.c b/builtin-update-index.c index 1ac613a..5b2278e 100644 --- a/builtin-update-index.c +++ b/builtin-update-index.c @@ -121,8 +121,10 @@ static int add_file_to_cache(const char *path) ce->ce_mode = create_ce_mode(S_IFREG | 0666); } - if (index_path(ce->sha1, path, &st, !info_only)) + if (index_path(ce->sha1, path, &st, !info_only)) { + free(ce); return -1; + } option = allow_add ? ADD_CACHE_OK_TO_ADD : 0; option |= allow_replace ? ADD_CACHE_OK_TO_REPLACE : 0; if (add_cache_entry(ce, option)) -- 1.5.0.33.gaf997-dirty ``` ## Linus Torvalds, 2007-02-16 17:00 Subject: Re: [PATCH] builtin-update-index.c (add_file_to_cache): Don't leak a cache entry. Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0702160858190.20368%40woody.linux-foundation.org In-Reply-To: <87mz3e327y.fsf@rho.meyering.net> ``` On Fri, 16 Feb 2007, Jim Meyering wrote: > > - if (index_path(ce->sha1, path, &st, !info_only)) > + if (index_path(ce->sha1, path, &st, !info_only)) { > + free(ce); > return -1; > + } Well, the only user of this does: if (add_file_to_cache(p)) die("Unable to process file %s", path); so the leak is very shortlived ;) Linus ``` ## Jim Meyering, 2007-02-18 09:10 Subject: Re: [PATCH] builtin-update-index.c (add_file_to_cache): Don't leak a cache entry. Message-ID: <87bqjrvnk2.fsf@rho.meyering.net> URL: https://gitlist.dev/e/87bqjrvnk2.fsf%40rho.meyering.net In-Reply-To: ``` Linus Torvalds wrote: > On Fri, 16 Feb 2007, Jim Meyering wrote: >> >> - if (index_path(ce->sha1, path, &st, !info_only)) >> + if (index_path(ce->sha1, path, &st, !info_only)) { >> + free(ce); >> return -1; >> + } > > Well, the only user of this does: > > if (add_file_to_cache(p)) > die("Unable to process file %s", path); > > so the leak is very shortlived ;) True... for now, and probably for ever after, but what if? However, the goal in plugging a leak like this is not to avoid wasting memory, but rather to avoid spending time investigating it, again and again. It's analogous to making your code compile warning-free. When a new warning/leak pops up, it's easy to spot (in case it really does matter), not obscured by lots of older ones. There are other ways to mark a leak as ignorable, but this method works across all leak-detecting tools. If you'd prefer to avoid the cost of a technically-unnecessary free, give it a different name, so it can be a no-op most of the time, and "free" only when compiled in leak-checking mode. ```