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

From: Yasushi SHOJI <yashi@atmark-techno.com>
Date: Aug 21, 2005, 07:14 UTC
Message-ID: <87irxzhid3.wl@mail2.atmark-techno.com>
In-Reply-To: <7v3bpapkmr.fsf@assigned-by-dhcp.cox.net>

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

Show 14 quoted lines

> 
> Yasushi SHOJI <yashi@atmark-techno.com> 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.

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 <yashi@atmark-techno.com>

---

 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;