{"thread":{"id":"38954","subject":"[PATCHv3 6/6] entry.c: fix a memleak","startedAt":"2015-03-31T01:22:05Z","lastAt":"2015-03-31T19:12:51Z","messageCount":16,"participants":["Stefan Beller","Eric Sunshine","Jeff King","Torsten Bögershausen","Junio C Hamano","John Keeping"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"258716","messageId":"1427764931-27745-1-git-send-email-sbeller@google.com","threadId":"38954","inReplyTo":null,"subject":"[PATCHv3 0/6] Memory leaks once again","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-31T01:22:05Z","receivedAt":"2015-03-31T01:22:05Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"I changed the patches based on Erics feedback, all of them look sane to me now.\n\nI still retain 2 patches which I need to convince myself are reasonable to send\nto the list though.\n\nJohn Keeping (1):\n entry.c: fix a memleak\n\nStefan Beller (5):\n shallow: fix a memleak\n line-log.c: fix a memleak\n line-log.c: fix a memleak\n wt-status.c: fix a memleak\n pack-bitmap.c: fix a memleak\n\n line-log.c | 4 ++++\n pack-bitmap.c | 2 ++\n shallow.c | 5 +++--\n streaming.c | 5 ++++-\n wt-status.c | 1 +\n 5 files changed, 14 insertions(+), 3 deletions(-)\n\n-- \n2.3.0.81.gc37f363\n"},{"id":"258718","messageId":"1427764931-27745-2-git-send-email-sbeller@google.com","threadId":"38954","inReplyTo":"1427764931-27745-1-git-send-email-sbeller@google.com","subject":"[PATCHv3 1/6] shallow: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-31T01:22:06Z","receivedAt":"2015-03-31T01:22:06Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n shallow.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/shallow.c b/shallow.c\nindex d8bf40a..11d5c4e 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -412,11 +412,12 @@ static void paint_down(struct paint_info *info, const unsigned char *sha1,\n \tstruct commit_list *head = NULL;\n \tint bitmap_nr = (info->nr_bits + 31) / 32;\n \tint bitmap_size = bitmap_nr * sizeof(uint32_t);\n-\tuint32_t *tmp = xmalloc(bitmap_size); /* to be freed before return */\n-\tuint32_t *bitmap = paint_alloc(info);\n+\tuint32_t *tmp, *bitmap;\n \tstruct commit *c = lookup_commit_reference_gently(sha1, 1);\n \tif (!c)\n \t\treturn;\n+\ttmp = xmalloc(bitmap_size); /* to be freed before return */\n+\tbitmap = paint_alloc(info);\n \tmemset(bitmap, 0, bitmap_size);\n \tbitmap[id / 32] |= (1 << (id % 32));\n \tcommit_list_insert(c, &head);\n-- \n2.3.0.81.gc37f363\n"},{"id":"258719","messageId":"1427764931-27745-3-git-send-email-sbeller@google.com","threadId":"38954","inReplyTo":"1427764931-27745-1-git-send-email-sbeller@google.com","subject":"[PATCHv3 2/6] line-log.c: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-31T01:22:07Z","receivedAt":"2015-03-31T01:22:07Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The `filepair` is assigned new memory with any iteration via\nprocess_diff_filepair, so free it before the current iteration ends.\n\nSigned-off-by: Stefan Beller \n---\n line-log.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/line-log.c b/line-log.c\nindex a490efe..b43ac58 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -1099,6 +1099,7 @@ static int process_all_files(struct line_log_data **range_out,\n \t\t\trg->pair = diff_filepair_dup(queue->queue[i]);\n \t\t\tmemcpy(&rg->diff, pairdiff, sizeof(struct diff_ranges));\n \t\t}\n+\t\tfree(pairdiff);\n \t}\n \n \treturn changed;\n-- \n2.3.0.81.gc37f363\n"},{"id":"258717","messageId":"1427764931-27745-4-git-send-email-sbeller@google.com","threadId":"38954","inReplyTo":"1427764931-27745-1-git-send-email-sbeller@google.com","subject":"[PATCHv3 3/6] line-log.c: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-31T01:22:08Z","receivedAt":"2015-03-31T01:22:08Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n line-log.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/line-log.c b/line-log.c\nindex b43ac58..db6e58d 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -1129,6 +1129,9 @@ static int process_ranges_ordinary_commit(struct rev_info *rev, struct commit *c\n \tchanged = process_all_files(&parent_range, rev, &queue, range);\n \tif (parent)\n \t\tadd_line_range(rev, parent, parent_range);\n+\telse\n+\t\tfree_line_log_data(parent_range);\n+\n \treturn changed;\n }\n \n-- \n2.3.0.81.gc37f363\n"},{"id":"258720","messageId":"1427764931-27745-5-git-send-email-sbeller@google.com","threadId":"38954","inReplyTo":"1427764931-27745-1-git-send-email-sbeller@google.com","subject":"[PATCHv3 4/6] wt-status.c: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-31T01:22:09Z","receivedAt":"2015-03-31T01:22:09Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In any code path of shorten_unambiguous_ref the return value is a\nxstrdup(some string), so it is safe to free the variable `base`\nin any codepath.\n\nSigned-off-by: Stefan Beller \n---\n wt-status.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 853419f..38cb165 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1550,6 +1550,7 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n \tbase = shorten_unambiguous_ref(base, 0);\n \tcolor_fprintf(s->fp, header_color, \"...\");\n \tcolor_fprintf(s->fp, branch_color_remote, \"%s\", base);\n+\tfree((char *)base);\n \n \tif (!upstream_is_gone && !num_ours && !num_theirs) {\n \t\tfputc(s->null_termination ? '\\0' : '\\n', s->fp);\n-- \n2.3.0.81.gc37f363\n"},{"id":"258721","messageId":"1427764931-27745-6-git-send-email-sbeller@google.com","threadId":"38954","inReplyTo":"1427764931-27745-1-git-send-email-sbeller@google.com","subject":"[PATCHv3 5/6] pack-bitmap.c: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-31T01:22:10Z","receivedAt":"2015-03-31T01:22:10Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n pack-bitmap.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 365f9d9..62a98cc 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -986,6 +986,8 @@ void test_bitmap_walk(struct rev_info *revs)\n \t\tfprintf(stderr, \"OK!\\n\");\n \telse\n \t\tfprintf(stderr, \"Mismatch!\\n\");\n+\n+\tfree(result);\n }\n \n static int rebuild_bitmap(uint32_t *reposition,\n-- \n2.3.0.81.gc37f363\n"},{"id":"258715","messageId":"1427764931-27745-7-git-send-email-sbeller@google.com","threadId":"38954","inReplyTo":"1427764931-27745-1-git-send-email-sbeller@google.com","subject":"[PATCHv3 6/6] entry.c: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-31T01:22:11Z","receivedAt":"2015-03-31T01:22:11Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: John Keeping \n\nstream_blob_to_fd() always frees the filter now, so there is no memory\nleak in entry.c:152 just before the `goto finish`.\n\nSigned-off-by: John Keeping \nSigned-off-by: Stefan Beller \n---\n \n I added Johns signoff here tentatively, John can you confirm the sign off?\n \n streaming.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/streaming.c b/streaming.c\nindex 2ff036a..811fcc2 100644\n--- a/streaming.c\n+++ b/streaming.c\n@@ -507,8 +507,11 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1, struct stream_filter *f\n \tint result = -1;\n \n \tst = open_istream(sha1, &type, &sz, filter);\n-\tif (!st)\n+\tif (!st) {\n+\t\tif (filter)\n+\t\t\tfree_stream_filter(filter);\n \t\treturn result;\n+\t}\n \tif (type != OBJ_BLOB)\n \t\tgoto close_and_exit;\n \tfor (;;) {\n-- \n2.3.0.81.gc37f363\n"},{"id":"258723","messageId":"CAPig+cRi9L+bEJsjFdihLQJjb38Bg500=sZ+w4FubR5f3=n-bQ@mail.gmail.com","threadId":"38954","inReplyTo":"1427764931-27745-2-git-send-email-sbeller@google.com","subject":"Re: [PATCHv3 1/6] shallow: fix a memleak","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-03-31T02:52:58Z","receivedAt":"2015-03-31T02:52:58Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 30, 2015 at 9:22 PM, Stefan Beller wrote:\n> shallow: fix a memleak\n> Signed-off-by: Stefan Beller \n> ---\n> diff --git a/shallow.c b/shallow.c\n> index d8bf40a..11d5c4e 100644\n> --- a/shallow.c\n> +++ b/shallow.c\n> @@ -412,11 +412,12 @@ static void paint_down(struct paint_info *info, const unsigned char *sha1,\n> struct commit_list *head = NULL;\n> int bitmap_nr = (info->nr_bits + 31) / 32;\n> int bitmap_size = bitmap_nr * sizeof(uint32_t);\n> - uint32_t *tmp = xmalloc(bitmap_size); /* to be freed before return */\n> - uint32_t *bitmap = paint_alloc(info);\n> + uint32_t *tmp, *bitmap;\n> struct commit *c = lookup_commit_reference_gently(sha1, 1);\n> if (!c)\n> return;\n> + tmp = xmalloc(bitmap_size); /* to be freed before return */\n> + bitmap = paint_alloc(info);\n\nYou've made two unrelated changes in this patch, only one of which is\na fix for the memory leak mentioned by the commit message.\n\nThe 'tmp' change fixes the memory leak. The 'bitmap' change, however,\nmerely avoids doing work unnecessarily in the \"early return\" case.\nSince 'bitmap' gets released by the caller of paint_down() when the\npaint_info structure is freed, it is misleading to claim that that\nparticular change is a \"memleak\" fix.\n\nEither split the two changes into separate patches or augment the\ncommit message to say something along the lines of:\n\n While here, also avoid unnecessarily allocating 'bitmap' within\n paint_info in the early-return case.\n\n> memset(bitmap, 0, bitmap_size);\n> bitmap[id / 32] |= (1 << (id % 32));\n> commit_list_insert(c, &head);\n> --\n> 2.3.0.81.gc37f363\n"},{"id":"258727","messageId":"20150331033202.GB842@peff.net","threadId":"38954","inReplyTo":"1427764931-27745-6-git-send-email-sbeller@google.com","subject":"Re: [PATCHv3 5/6] pack-bitmap.c: fix a memleak","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-31T03:32:02Z","receivedAt":"2015-03-31T03:32:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 30, 2015 at 06:22:10PM -0700, Stefan Beller wrote:\n\n> Signed-off-by: Stefan Beller \n> ---\n> pack-bitmap.c | 2 ++\n> 1 file changed, 2 insertions(+)\n> \n> diff --git a/pack-bitmap.c b/pack-bitmap.c\n> index 365f9d9..62a98cc 100644\n> --- a/pack-bitmap.c\n> +++ b/pack-bitmap.c\n> @@ -986,6 +986,8 @@ void test_bitmap_walk(struct rev_info *revs)\n> \t\tfprintf(stderr, \"OK!\\n\");\n> \telse\n> \t\tfprintf(stderr, \"Mismatch!\\n\");\n> +\n> +\tfree(result);\n\nI think this one is fine. I see you dropped the other bits from\nload_bitmap_entries_v1. There are definitely some leaks there in the\nerror case, and I left some notes in the other thread.\n\nI don't think those leaks are all that critical (we only load the\nbitmaps one per process, and the memory persists if we succeed, so the\nworst case is that we spend the memory but do not get to use the\nbitmaps). But if your goal is making us valgrind-clean, they would\ncertainly count.\n\n-Peff\n"},{"id":"258729","messageId":"551A25B1.80309@web.de","threadId":"38954","inReplyTo":"1427764931-27745-4-git-send-email-sbeller@google.com","subject":"Re: [PATCHv3 3/6] line-log.c: fix a memleak","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-03-31T04:42:25Z","receivedAt":"2015-03-31T04:42:25Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 03/31/2015 03:22 AM, Stefan Beller wrote:\n> Signed-off-by: Stefan Beller \n> ---\n> line-log.c | 3 +++\n> 1 file changed, 3 insertions(+)\n>\nWould it make sense to join 2/6 and 3/6 into a single patch ?\n\nBoth fix the memleaks in the same file, and the\nheader can be \"fix memleaks\".\n"},{"id":"258731","messageId":"xmqqsiclsqlz.fsf@gitster.dls.corp.google.com","threadId":"38954","inReplyTo":"1427764931-27745-4-git-send-email-sbeller@google.com","subject":"Re: [PATCHv3 3/6] line-log.c: fix a memleak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-31T05:06:16Z","receivedAt":"2015-03-31T05:06:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Signed-off-by: Stefan Beller \n> ---\n> line-log.c | 3 +++\n> 1 file changed, 3 insertions(+)\n>\n> diff --git a/line-log.c b/line-log.c\n> index b43ac58..db6e58d 100644\n> --- a/line-log.c\n> +++ b/line-log.c\n> @@ -1129,6 +1129,9 @@ static int process_ranges_ordinary_commit(struct rev_info *rev, struct commit *c\n> \tchanged = process_all_files(&parent_range, rev, &queue, range);\n> \tif (parent)\n> \t\tadd_line_range(rev, parent, parent_range);\n> +\telse\n> +\t\tfree_line_log_data(parent_range);\n> +\n\nShould this be in the else clause? From a cursory scan\nadd-line-range feeds it to either line-log-data-merge or\nline-log-data-copy to create \"new\" that is a separate thing from the\nparent_range we are feeding it, and that copy is used by\nadd-decoration. If add-line-range does not take ownership of\nparent_range from us, shouldn't we be freeing it whether we called\nit or not???\n\n> \treturn changed;\n> }\n"},{"id":"258732","messageId":"551A323F.9030305@gmail.com","threadId":"38954","inReplyTo":"xmqqsiclsqlz.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCHv3 3/6] line-log.c: fix a memleak","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2015-03-31T05:35:59Z","receivedAt":"2015-03-31T05:35:59Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 30.03.2015 22:06, Junio C Hamano wrote:\n> If add-line-range does not take ownership of\n> parent_range from us, shouldn't we be freeing it whether we called\n> it or not???\n\nIn my understanding it does take ownership of it. (According to my code\nreview on friday)\n"},{"id":"258733","messageId":"xmqqoan9soym.fsf@gitster.dls.corp.google.com","threadId":"38954","inReplyTo":"1427764931-27745-7-git-send-email-sbeller@google.com","subject":"Re: [PATCHv3 6/6] entry.c: fix a memleak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-31T05:41:53Z","receivedAt":"2015-03-31T05:41:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> From: John Keeping \n>\n> stream_blob_to_fd() always frees the filter now, so there is no memory\n> leak in entry.c:152 just before the `goto finish`.\n>\n> Signed-off-by: John Keeping \n> Signed-off-by: Stefan Beller \n> ---\n> \n> I added Johns signoff here tentatively, John can you confirm the sign off?\n> \n> streaming.c | 5 ++++-\n> 1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/streaming.c b/streaming.c\n> index 2ff036a..811fcc2 100644\n> --- a/streaming.c\n> +++ b/streaming.c\n> @@ -507,8 +507,11 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1, struct stream_filter *f\n> \tint result = -1;\n> \n> \tst = open_istream(sha1, &type, &sz, filter);\n> -\tif (!st)\n> +\tif (!st) {\n> +\t\tif (filter)\n> +\t\t\tfree_stream_filter(filter);\n> \t\treturn result;\n> +\t}\n\nI think this one is fine. I see you moved the callsite to free this\nthing from entry.c down to streaming (stream_blob_to_fd()), which\nmeans you would need to adjust the title of the change, too.\n\nIt somewhat feels dirty that we free what the caller gave us, but\ngiven that closing a filtered stream frees the filter at the end,\nI think this is the right place to discard the filter. In essence,\nthe overall use of the API in this code is equivalent to\n\n\tfilter = ... prepare a filter ...\n if (open_istream(... filter) == OK) {\n\t\t... use filter ...\n close_istream(); /* which frees the filter */\n\t} else {\n \t/* we failed to free filter here without the patch */\n\t\tfree_stream_filter(filter);\n\t}\n\nso the patch makes sense.\n\nWe may want to teach free_stream_filter() that a request to free a\nNULL filter is OK, though. That would be a separate clean-up topic.\n\n\n> \tif (type != OBJ_BLOB)\n> \t\tgoto close_and_exit;\n> \tfor (;;) {\n"},{"id":"258734","messageId":"20150331083647.GA2248@serenity.lan","threadId":"38954","inReplyTo":"1427764931-27745-7-git-send-email-sbeller@google.com","subject":"Re: [PATCHv3 6/6] entry.c: fix a memleak","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2015-03-31T08:36:47Z","receivedAt":"2015-03-31T08:36:47Z","isPatch":false,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Mon, Mar 30, 2015 at 06:22:11PM -0700, Stefan Beller wrote:\n> From: John Keeping \n> \n> stream_blob_to_fd() always frees the filter now, so there is no memory\n> leak in entry.c:152 just before the `goto finish`.\n> \n> Signed-off-by: John Keeping \n> Signed-off-by: Stefan Beller \n> ---\n> \n> I added Johns signoff here tentatively, John can you confirm the sign off?\n\nSigned-off-by: John Keeping \n\n> streaming.c | 5 ++++-\n> 1 file changed, 4 insertions(+), 1 deletion(-)\n> \n> diff --git a/streaming.c b/streaming.c\n> index 2ff036a..811fcc2 100644\n> --- a/streaming.c\n> +++ b/streaming.c\n> @@ -507,8 +507,11 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1, struct stream_filter *f\n> \tint result = -1;\n> \n> \tst = open_istream(sha1, &type, &sz, filter);\n> -\tif (!st)\n> +\tif (!st) {\n> +\t\tif (filter)\n> +\t\t\tfree_stream_filter(filter);\n> \t\treturn result;\n> +\t}\n> \tif (type != OBJ_BLOB)\n> \t\tgoto close_and_exit;\n> \tfor (;;) {\n> -- \n> 2.3.0.81.gc37f363\n> \n"},{"id":"258765","messageId":"xmqqr3s5q9an.fsf@gitster.dls.corp.google.com","threadId":"38954","inReplyTo":"551A323F.9030305@gmail.com","subject":"Re: [PATCHv3 3/6] line-log.c: fix a memleak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-31T19:03:12Z","receivedAt":"2015-03-31T19:03:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On 30.03.2015 22:06, Junio C Hamano wrote:\n>> If add-line-range does not take ownership of\n>> parent_range from us, shouldn't we be freeing it whether we called\n>> it or not???\n>\n> In my understanding it does take ownership of it. (According to my code\n> review on friday)\n\nHmmm.\n\nadd_line_range() is given the \"parent_range\" as its third parameter,\ni.e. \"range\".\n\nIf there is nothing for the commit yet, it calls line_log_data_copy()\non it to make a deep copy and stores that in new, which is then used\nas the decoration for the commit---the copy is what is retained.\n\nI am not quite sure what the lifetime rule is for the original of\nthis deep copy, i.e. \"range\", aka \"parent_range\" of the caller.\nWho holds pointers into it after this function is done?\n"},{"id":"258766","messageId":"xmqqmw2tq8uk.fsf@gitster.dls.corp.google.com","threadId":"38954","inReplyTo":"20150331083647.GA2248@serenity.lan","subject":"Re: [PATCHv3 6/6] entry.c: fix a memleak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-31T19:12:51Z","receivedAt":"2015-03-31T19:12:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> On Mon, Mar 30, 2015 at 06:22:11PM -0700, Stefan Beller wrote:\n>> From: John Keeping \n>> \n>> stream_blob_to_fd() always frees the filter now, so there is no memory\n>> leak in entry.c:152 just before the `goto finish`.\n>> \n>> Signed-off-by: John Keeping \n>> Signed-off-by: Stefan Beller \n>> ---\n>> \n>> I added Johns signoff here tentatively, John can you confirm the sign off?\n>\n> Signed-off-by: John Keeping \n\nThanks. I'll queue with a retitle, as this no longer is about\nentry.c at all.\n\n>\n>> streaming.c | 5 ++++-\n>> 1 file changed, 4 insertions(+), 1 deletion(-)\n>> \n>> diff --git a/streaming.c b/streaming.c\n>> index 2ff036a..811fcc2 100644\n>> --- a/streaming.c\n>> +++ b/streaming.c\n>> @@ -507,8 +507,11 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1, struct stream_filter *f\n>> \tint result = -1;\n>> \n>> \tst = open_istream(sha1, &type, &sz, filter);\n>> -\tif (!st)\n>> +\tif (!st) {\n>> +\t\tif (filter)\n>> +\t\t\tfree_stream_filter(filter);\n>> \t\treturn result;\n>> +\t}\n>> \tif (type != OBJ_BLOB)\n>> \t\tgoto close_and_exit;\n>> \tfor (;;) {\n>> -- \n>> 2.3.0.81.gc37f363\n>> \n"}]}