{"thread":{"id":"38929","subject":"[PATCH 0/6] Memory leaks once again","startedAt":"2015-03-27T22:09:00Z","lastAt":"2015-03-27T22:12:36Z","messageCount":12,"participants":["Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"258599","messageId":"1427494150-8085-1-git-send-email-sbeller@google.com","threadId":"38929","inReplyTo":null,"subject":"[PATCH 0/6] Memory leaks once again","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-27T22:09:00Z","receivedAt":"2015-03-27T22:09:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Here comes another bunch of memory leaks fixed.\npatches 1-4 are safe bets, but 5 and 6 are not so.\n\nIn patch 5 I wonder if we need to fix more aggressively and\nin patch 6 I just know there is a leak but I have no idea how to\nactually fix it.\n\nStefan Beller (6):\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: fix a memleak\n WIP/RFC/entry.c: fix a memleak\n\n entry.c | 4 +++-\n line-log.c | 4 ++++\n pack-bitmap.c | 27 ++++++++++++++++++---------\n shallow.c | 4 ++--\n wt-status.c | 2 ++\n 5 files changed, 29 insertions(+), 12 deletions(-)\n\n-- \n2.3.0.81.gc37f363\n"},{"id":"258602","messageId":"1427494150-8085-2-git-send-email-sbeller@google.com","threadId":"38929","inReplyTo":"1427494150-8085-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/6] pack-bitmap: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-27T22:09:01Z","receivedAt":"2015-03-27T22:09:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"`recent_bitmaps` is allocated in the function load_bitmap_entries_v1\nand it is not passed into any function, so it's safe to free it before\nleaving that function.\n\nSigned-off-by: Stefan Beller \n---\n\nNotes:\n I wonder however if we need to free the actual bitmaps\n stored in the recent_bitmaps as well.\n\n pack-bitmap.c | 27 ++++++++++++++++++---------\n 1 file changed, 18 insertions(+), 9 deletions(-)\n\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 365f9d9..34823e9 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -213,7 +213,7 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)\n {\n \tstatic const size_t MAX_XOR_OFFSET = 160;\n \n-\tuint32_t i;\n+\tuint32_t i, ret = 0;\n \tstruct stored_bitmap **recent_bitmaps;\n \n \trecent_bitmaps = xcalloc(MAX_XOR_OFFSET, sizeof(struct stored_bitmap));\n@@ -232,24 +232,31 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)\n \t\tsha1 = nth_packed_object_sha1(index->pack, commit_idx_pos);\n \n \t\tbitmap = read_bitmap_1(index);\n-\t\tif (!bitmap)\n-\t\t\treturn -1;\n+\t\tif (!bitmap) {\n+\t\t\tret = -1;\n+\t\t\tgoto out;\n+\t\t}\n \n-\t\tif (xor_offset > MAX_XOR_OFFSET || xor_offset > i)\n-\t\t\treturn error(\"Corrupted bitmap pack index\");\n+\t\tif (xor_offset > MAX_XOR_OFFSET || xor_offset > i) {\n+\t\t\tret = error(\"Corrupted bitmap pack index\");\n+\t\t\tgoto out;\n+\t\t}\n \n \t\tif (xor_offset > 0) {\n \t\t\txor_bitmap = recent_bitmaps[(i - xor_offset) % MAX_XOR_OFFSET];\n \n-\t\t\tif (xor_bitmap == NULL)\n-\t\t\t\treturn error(\"Invalid XOR offset in bitmap pack index\");\n+\t\t\tif (xor_bitmap == NULL) {\n+\t\t\t\tret = error(\"Invalid XOR offset in bitmap pack index\");\n+\t\t\t\tgoto out;\n+\t\t\t}\n \t\t}\n \n \t\trecent_bitmaps[i % MAX_XOR_OFFSET] = store_bitmap(\n \t\t\tindex, bitmap, sha1, xor_bitmap, flags);\n \t}\n-\n-\treturn 0;\n+out:\n+\tfree(recent_bitmaps);\n+\treturn ret;\n }\n \n static char *pack_bitmap_filename(struct packed_git *p)\n@@ -986,6 +993,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":"258601","messageId":"1427494150-8085-3-git-send-email-sbeller@google.com","threadId":"38929","inReplyTo":"1427494150-8085-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/6] shallow: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-27T22:09:02Z","receivedAt":"2015-03-27T22:09:02Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n shallow.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/shallow.c b/shallow.c\nindex d8bf40a..f8b0458 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -416,7 +416,7 @@ static void paint_down(struct paint_info *info, const unsigned char *sha1,\n \tuint32_t *bitmap = paint_alloc(info);\n \tstruct commit *c = lookup_commit_reference_gently(sha1, 1);\n \tif (!c)\n-\t\treturn;\n+\t\tgoto out;\n \tmemset(bitmap, 0, bitmap_size);\n \tbitmap[id / 32] |= (1 << (id % 32));\n \tcommit_list_insert(c, &head);\n@@ -471,7 +471,7 @@ static void paint_down(struct paint_info *info, const unsigned char *sha1,\n \t\tif (o && o->type == OBJ_COMMIT)\n \t\t\to->flags &= ~SEEN;\n \t}\n-\n+out:\n \tfree(tmp);\n }\n \n-- \n2.3.0.81.gc37f363\n"},{"id":"258600","messageId":"1427494150-8085-4-git-send-email-sbeller@google.com","threadId":"38929","inReplyTo":"1427494150-8085-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/6] line-log.c: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-27T22:09:03Z","receivedAt":"2015-03-27T22:09:03Z","isPatch":true,"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":"258609","messageId":"1427494150-8085-5-git-send-email-sbeller@google.com","threadId":"38929","inReplyTo":"1427494150-8085-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/6] shallow: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-27T22:09:04Z","receivedAt":"2015-03-27T22:09:04Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n shallow.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/shallow.c b/shallow.c\nindex d8bf40a..f8b0458 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -416,7 +416,7 @@ static void paint_down(struct paint_info *info, const unsigned char *sha1,\n \tuint32_t *bitmap = paint_alloc(info);\n \tstruct commit *c = lookup_commit_reference_gently(sha1, 1);\n \tif (!c)\n-\t\treturn;\n+\t\tgoto out;\n \tmemset(bitmap, 0, bitmap_size);\n \tbitmap[id / 32] |= (1 << (id % 32));\n \tcommit_list_insert(c, &head);\n@@ -471,7 +471,7 @@ static void paint_down(struct paint_info *info, const unsigned char *sha1,\n \t\tif (o && o->type == OBJ_COMMIT)\n \t\t\to->flags &= ~SEEN;\n \t}\n-\n+out:\n \tfree(tmp);\n }\n \n-- \n2.3.0.81.gc37f363\n"},{"id":"258608","messageId":"1427494150-8085-6-git-send-email-sbeller@google.com","threadId":"38929","inReplyTo":"1427494150-8085-1-git-send-email-sbeller@google.com","subject":"[PATCH 3/6] line-log.c: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-27T22:09:05Z","receivedAt":"2015-03-27T22:09:05Z","isPatch":true,"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":"258607","messageId":"1427494150-8085-7-git-send-email-sbeller@google.com","threadId":"38929","inReplyTo":"1427494150-8085-1-git-send-email-sbeller@google.com","subject":"[PATCH 4/6] line-log.c: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-27T22:09:06Z","receivedAt":"2015-03-27T22:09:06Z","isPatch":true,"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":"258606","messageId":"1427494150-8085-8-git-send-email-sbeller@google.com","threadId":"38929","inReplyTo":"1427494150-8085-1-git-send-email-sbeller@google.com","subject":"[PATCH 4/6] wt-status.c: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-27T22:09:07Z","receivedAt":"2015-03-27T22:09:07Z","isPatch":true,"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 | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 853419f..9d2a349 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1576,6 +1576,8 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n \n \tcolor_fprintf(s->fp, header_color, \"]\");\n \tfputc(s->null_termination ? '\\0' : '\\n', s->fp);\n+\n+\tfree((char *)base);\n }\n \n void wt_shortstatus_print(struct wt_status *s)\n-- \n2.3.0.81.gc37f363\n"},{"id":"258604","messageId":"1427494150-8085-9-git-send-email-sbeller@google.com","threadId":"38929","inReplyTo":"1427494150-8085-1-git-send-email-sbeller@google.com","subject":"[PATCH 5/6] pack-bitmap: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-27T22:09:08Z","receivedAt":"2015-03-27T22:09:08Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"`recent_bitmaps` is allocated in the function load_bitmap_entries_v1\nand it is not passed into any function, so it's safe to free it before\nleaving that function.\n\nSigned-off-by: Stefan Beller \n---\n\nNotes:\n I wonder however if we need to free the actual bitmaps\n stored in the recent_bitmaps as well.\n\n pack-bitmap.c | 27 ++++++++++++++++++---------\n 1 file changed, 18 insertions(+), 9 deletions(-)\n\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 365f9d9..34823e9 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -213,7 +213,7 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)\n {\n \tstatic const size_t MAX_XOR_OFFSET = 160;\n \n-\tuint32_t i;\n+\tuint32_t i, ret = 0;\n \tstruct stored_bitmap **recent_bitmaps;\n \n \trecent_bitmaps = xcalloc(MAX_XOR_OFFSET, sizeof(struct stored_bitmap));\n@@ -232,24 +232,31 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)\n \t\tsha1 = nth_packed_object_sha1(index->pack, commit_idx_pos);\n \n \t\tbitmap = read_bitmap_1(index);\n-\t\tif (!bitmap)\n-\t\t\treturn -1;\n+\t\tif (!bitmap) {\n+\t\t\tret = -1;\n+\t\t\tgoto out;\n+\t\t}\n \n-\t\tif (xor_offset > MAX_XOR_OFFSET || xor_offset > i)\n-\t\t\treturn error(\"Corrupted bitmap pack index\");\n+\t\tif (xor_offset > MAX_XOR_OFFSET || xor_offset > i) {\n+\t\t\tret = error(\"Corrupted bitmap pack index\");\n+\t\t\tgoto out;\n+\t\t}\n \n \t\tif (xor_offset > 0) {\n \t\t\txor_bitmap = recent_bitmaps[(i - xor_offset) % MAX_XOR_OFFSET];\n \n-\t\t\tif (xor_bitmap == NULL)\n-\t\t\t\treturn error(\"Invalid XOR offset in bitmap pack index\");\n+\t\t\tif (xor_bitmap == NULL) {\n+\t\t\t\tret = error(\"Invalid XOR offset in bitmap pack index\");\n+\t\t\t\tgoto out;\n+\t\t\t}\n \t\t}\n \n \t\trecent_bitmaps[i % MAX_XOR_OFFSET] = store_bitmap(\n \t\t\tindex, bitmap, sha1, xor_bitmap, flags);\n \t}\n-\n-\treturn 0;\n+out:\n+\tfree(recent_bitmaps);\n+\treturn ret;\n }\n \n static char *pack_bitmap_filename(struct packed_git *p)\n@@ -986,6 +993,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":"258603","messageId":"1427494150-8085-10-git-send-email-sbeller@google.com","threadId":"38929","inReplyTo":"1427494150-8085-1-git-send-email-sbeller@google.com","subject":"[PATCH 5/6] wt-status.c: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-27T22:09:09Z","receivedAt":"2015-03-27T22:09:09Z","isPatch":true,"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 | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 853419f..9d2a349 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1576,6 +1576,8 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n \n \tcolor_fprintf(s->fp, header_color, \"]\");\n \tfputc(s->null_termination ? '\\0' : '\\n', s->fp);\n+\n+\tfree((char *)base);\n }\n \n void wt_shortstatus_print(struct wt_status *s)\n-- \n2.3.0.81.gc37f363\n"},{"id":"258605","messageId":"1427494150-8085-11-git-send-email-sbeller@google.com","threadId":"38929","inReplyTo":"1427494150-8085-1-git-send-email-sbeller@google.com","subject":"[PATCH 6/6] WIP/RFC/entry.c: fix a memleak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-27T22:09:10Z","receivedAt":"2015-03-27T22:09:10Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"I observe that filter is going out of scope, but the\nimplementation proposed in this patch produces just a\ncrash instead of any helpful fix.\n\nSigned-off-by: Stefan Beller \n---\n entry.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/entry.c b/entry.c\nindex 1eda8e9..5383001 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -152,8 +152,10 @@ static int write_entry(struct cache_entry *ce,\n \t\tif (filter &&\n \t\t !streaming_write_entry(ce, path, filter,\n \t\t\t\t\t state, to_tempfile,\n-\t\t\t\t\t &fstat_done, &st))\n+\t\t\t\t\t &fstat_done, &st)) {\n+\t\t\tfree_stream_filter(filter);\n \t\t\tgoto finish;\n+\t\t}\n \t}\n \n \tswitch (ce_mode_s_ifmt) {\n-- \n2.3.0.81.gc37f363\n"},{"id":"258610","messageId":"CAGZ79kZ8=HoF0US0988oZxTyubjJO4vGYCtrUPX_x6gE0M2_Vw@mail.gmail.com","threadId":"38929","inReplyTo":"1427494150-8085-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH 0/6] Memory leaks once again","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-03-27T22:12:36Z","receivedAt":"2015-03-27T22:12:36Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Mar 27, 2015 at 3:09 PM, Stefan Beller wrote:\n> Here comes another bunch of memory leaks fixed.\n> patches 1-4 are safe bets, but 5 and 6 are not so.\n>\n> In patch 5 I wonder if we need to fix more aggressively and\n> in patch 6 I just know there is a leak but I have no idea how to\n> actually fix it.\n>\n\nAnd I sent 12 instead of 6 patches because I needed to fine tune the\norder of the patch set and I forgot to delete the un-tuned version.\n"}]}