| Message ID | pull.1152.v2.git.1645320591.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix a couple small leaks in merge-ort | expand |
On Sun, Feb 20, 2022 at 01:29:49AM +0000, Elijah Newren via GitGitGadget wrote: > Off-list, Ævar reported a few small leaks in merge-ort to me that I missed > previously. Here's a couple fixes. > > Changes since v1: > > * Simplified patch 1 a bit as per Ævar's suggestion Thanks; both look good. I left a comment about an issue unrelated to this series on the first patch. But this looks ready to go to my eyes. Thanks, Taylor
Off-list, Ævar reported a few small leaks in merge-ort to me that I missed previously. Here's a couple fixes. Changes since v1: * Simplified patch 1 a bit as per Ævar's suggestion Elijah Newren (2): merge-ort: fix small memory leak in detect_and_process_renames() merge-ort: fix small memory leak in unique_path() merge-ort.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) base-commit: e2ac9141e64e2cd3e690d1b5fc848949827c09b4 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1152%2Fnewren%2Fmerge-ort-leak-fixes-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1152/newren/merge-ort-leak-fixes-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1152 Range-diff vs v1: 1: f0308de28e4 ! 1: f1f7fc97fe2 merge-ort: fix small memory leak in detect_and_process_renames() @@ Commit message free(combined.queue); } - The problem is that sometimes even when there are pairs, none of them are - necessary. Instead of checking combined.nr, we should check - combined.alloc. Doing so fixes the following memory leak, as reported - by valgrind: + The problem is that sometimes even when there are pairs, none of them + are necessary. Instead of checking combined.nr, just remove the + if-check; free() knows to skip NULL pointers. This change fixes the + following memory leak, as reported by valgrind: ==PID== 192 bytes in 1 blocks are definitely lost in loss record 107 of 134 ==PID== at 0xADDRESS: malloc @@ Commit message Signed-off-by: Elijah Newren <newren@gmail.com> ## merge-ort.c ## +@@ merge-ort.c: static int detect_and_process_renames(struct merge_options *opt, + struct tree *side1, + struct tree *side2) + { +- struct diff_queue_struct combined; ++ struct diff_queue_struct combined = { 0 }; + struct rename_info *renames = &opt->priv->renames; +- int need_dir_renames, s, clean = 1; ++ int need_dir_renames, s, i, clean = 1; + unsigned detection_run = 0; + +- memset(&combined, 0, sizeof(combined)); + if (!possible_renames(renames)) + goto cleanup; + @@ merge-ort.c: simple_cleanup: free(renames->pairs[s].queue); DIFF_QUEUE_CLEAR(&renames->pairs[s]); } - if (combined.nr) { -+ if (combined.alloc) { - int i; - for (i = 0; i < combined.nr; i++) - pool_diff_free_filepair(&opt->priv->pool, +- int i; +- for (i = 0; i < combined.nr; i++) +- pool_diff_free_filepair(&opt->priv->pool, +- combined.queue[i]); +- free(combined.queue); +- } ++ for (i = 0; i < combined.nr; i++) ++ pool_diff_free_filepair(&opt->priv->pool, combined.queue[i]); ++ free(combined.queue); + + return clean; + } 2: 73bc1e5c5df = 2: 69fb932c21d merge-ort: fix small memory leak in unique_path()