Re: [patch] possible memory leak in diff.c::diff_free_filepair()

Yasushi SHOJI [off-list ref] writes:

parepare_temp_file() and diff_populate_filespec() has a lot in
similarity. so it'd be nice to refactor some. and re-introduce
diff_free_filespec_data() and call right after prep_temp_blob() in
prepare_temp_file().

Another thing that may (or may not) help would be to move that
prepare_temp_file() and stuff to happen in the forked child
process instead of the parent, so that we do not have to worry
about freeing the buffer; it has been some time since I worked
on that part of the code so this may not be an option to make
things easier.

Junio, did you also mean to clean-up these functions when you said in
the thread of "Re: gitweb - option to disable rename detection"?

No.  I was talking about cleaning up the similar option parsing
code, and the call into diffcore_std() which takes more
arguments every time a new option is added to the family.

At Mon, 15 Aug 2005 21:32:12 -0700,
Junio C Hamano wrote:

Yasushi SHOJI [off-list ref] writes:

quoted

parepare_temp_file() and diff_populate_filespec() has a lot in
similarity. so it'd be nice to refactor some. and re-introduce
diff_free_filespec_data() and call right after prep_temp_blob() in
prepare_temp_file().

Another thing that may (or may not) help would be to move that
prepare_temp_file() and stuff to happen in the forked child
process instead of the parent, so that we do not have to worry
about freeing the buffer; it has been some time since I worked
on that part of the code so this may not be an option to make
things easier.

well, I couldn't figure out how to do it in child.  because the parent
need to know temp files in order to delete after child used it.

anyway, here is a patch to fix the problem in the simplest way.

regards,
--
          yashi

-----------
Make diff_flush a bit more system friendly.

This seems to be the simplest way to fix the peak time
high memory usage in diff_flush.  Don't wait all iteration
to free each filepair.

Signed-off-by: Yasushi SHOJI <redacted>


---

 diff.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

e0f7f97ddc29a9172c2f6c1221eca936aa80835c

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c

@@ -1010,9 +1010,8 @@ void diff_flush(int diff_output_style, i
 			diff_flush_name(p, line_termination);
 			break;
 		}
-	}
-	for (i = 0; i < q->nr; i++)
 		diff_free_filepair(q->queue[i]);
+	}
 	free(q->queue);
 	q->queue = NULL;
 	q->nr = q->alloc = 0;