Skip to content

Fix a couple small leaks in merge-ort

Elijah Newren via GitGitGadget git 2 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest2 patches 5 repliesIn review
  2. v12 patches 4 repliesSuperseded

Changes in v2 from the cover letter

* Simplified patch 1 a bit as per Ævar's suggestion

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2merge-ort: fix small memory leak in detect_and_process_renames()merge-ort.c+5 -104 replies
2/2merge-ort: fix small memory leak in unique_path()merge-ort.c+12 -90 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.

Read the v2 thread

Cover letter [PATCH v2 0/2] Fix a couple small leaks in merge-ort

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 <[email protected]>
      
       ## 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()

Signature · 1 line
gitgitgadget