All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yasushi SHOJI <yashi@atmark-techno.com>
To: Junio C Hamano <junkio@cox.net>
Cc: git@vger.kernel.org
Subject: Re: [patch] possible memory leak in diff.c::diff_free_filepair()
Date: Sun, 21 Aug 2005 16:14:16 +0900	[thread overview]
Message-ID: <87irxzhid3.wl@mail2.atmark-techno.com> (raw)
In-Reply-To: <7v3bpapkmr.fsf@assigned-by-dhcp.cox.net>

At Mon, 15 Aug 2005 21:32:12 -0700,
Junio C Hamano wrote:
> 
> 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;

      reply	other threads:[~2005-08-21  7:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-08-13 10:58 [patch] possible memory leak in diff.c::diff_free_filepair() Yasushi SHOJI
2005-08-13 19:30 ` Junio C Hamano
2005-08-13 21:09   ` Yasushi SHOJI
2005-08-13 21:31     ` Junio C Hamano
2005-08-16  3:05       ` Yasushi SHOJI
2005-08-16  4:32         ` Junio C Hamano
2005-08-21  7:14           ` Yasushi SHOJI [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87irxzhid3.wl@mail2.atmark-techno.com \
    --to=yashi@atmark-techno.com \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.