{"thread":{"id":"6843","subject":"[PATCH] builtin-update-index.c (add_file_to_cache): Don't leak a cache entry.","startedAt":"2007-02-16T09:01:53Z","lastAt":"2007-02-18T09:10:21Z","messageCount":3,"participants":["Jim Meyering","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"34789","messageId":"87mz3e327y.fsf@rho.meyering.net","threadId":"6843","inReplyTo":null,"subject":"[PATCH] builtin-update-index.c (add_file_to_cache): Don't leak a cache entry.","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2007-02-16T09:01:53Z","receivedAt":"2007-02-16T09:01:53Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"\nSigned-off-by: Jim Meyering <jim@meyering.net>\n---\n builtin-update-index.c |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-update-index.c b/builtin-update-index.c\nindex 1ac613a..5b2278e 100644\n--- a/builtin-update-index.c\n+++ b/builtin-update-index.c\n@@ -121,8 +121,10 @@ static int add_file_to_cache(const char *path)\n \t\t\tce->ce_mode = create_ce_mode(S_IFREG | 0666);\n \t}\n\n-\tif (index_path(ce->sha1, path, &st, !info_only))\n+\tif (index_path(ce->sha1, path, &st, !info_only)) {\n+\t\tfree(ce);\n \t\treturn -1;\n+\t}\n \toption = allow_add ? ADD_CACHE_OK_TO_ADD : 0;\n \toption |= allow_replace ? ADD_CACHE_OK_TO_REPLACE : 0;\n \tif (add_cache_entry(ce, option))\n--\n1.5.0.33.gaf997-dirty\n"},{"id":"34812","messageId":"Pine.LNX.4.64.0702160858190.20368@woody.linux-foundation.org","threadId":"6843","inReplyTo":"87mz3e327y.fsf@rho.meyering.net","subject":"Re: [PATCH] builtin-update-index.c (add_file_to_cache): Don't leak a cache entry.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-02-16T17:00:34Z","receivedAt":"2007-02-16T17:00:34Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 16 Feb 2007, Jim Meyering wrote:\n> \n> -\tif (index_path(ce->sha1, path, &st, !info_only))\n> +\tif (index_path(ce->sha1, path, &st, !info_only)) {\n> +\t\tfree(ce);\n>  \t\treturn -1;\n> +\t}\n\nWell, the only user of this does:\n\n\tif (add_file_to_cache(p))\n\t\tdie(\"Unable to process file %s\", path);\n\nso the leak is very shortlived ;)\n\n\t\tLinus\n"},{"id":"34947","messageId":"87bqjrvnk2.fsf@rho.meyering.net","threadId":"6843","inReplyTo":"Pine.LNX.4.64.0702160858190.20368@woody.linux-foundation.org","subject":"Re: [PATCH] builtin-update-index.c (add_file_to_cache): Don't leak a cache entry.","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2007-02-18T09:10:21Z","receivedAt":"2007-02-18T09:10:21Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> wrote:\n> On Fri, 16 Feb 2007, Jim Meyering wrote:\n>>\n>> -\tif (index_path(ce->sha1, path, &st, !info_only))\n>> +\tif (index_path(ce->sha1, path, &st, !info_only)) {\n>> +\t\tfree(ce);\n>>  \t\treturn -1;\n>> +\t}\n>\n> Well, the only user of this does:\n>\n> \tif (add_file_to_cache(p))\n> \t\tdie(\"Unable to process file %s\", path);\n>\n> so the leak is very shortlived ;)\n\nTrue... for now, and probably for ever after, but what if?  However,\nthe goal in plugging a leak like this is not to avoid wasting memory,\nbut rather to avoid spending time investigating it, again and again.\nIt's analogous to making your code compile warning-free.  When a new\nwarning/leak pops up, it's easy to spot (in case it really does matter),\nnot obscured by lots of older ones.\n\nThere are other ways to mark a leak as ignorable, but this method works\nacross all leak-detecting tools.\n\nIf you'd prefer to avoid the cost of a technically-unnecessary free,\ngive it a different name, so it can be a no-op most of the time, and\n\"free\" only when compiled in leak-checking mode.\n"}]}