git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[AGGREGATED PATCH] Fix in-place editing functions in convert.c

From
Pierre Habouzit <madcoder@debian.org>
Date
Oct 5, 2007, 08:11 UTC
Message-ID
<20071005085522.32EFF1E16E@madism.org>
In-Reply-To
<20071005082026.GE19879@artemis.corp>
* crlf_to_git and ident_to_git:
  Don't grow the buffer if there is enough space in the first place.
  As a side effect, when the editing is done "in place", we don't grow, so
  the buffer pointer doesn't changes, and `src' isn't invalidated anymore.
  Thanks to Bernt Hansen for the bug report.
* apply_filter:
  Fix memory leak due to fake in-place editing that didn't collected the
  old buffer when the filter succeeds. Also a cosmetic fix.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

This patch is on top of master, and supersedes both patch I sent before. Following dscho's remark, I only grow the buffer if they aren't big enough in the first place, which ensures that buffers are not touched if edited in place.

 convert.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/convert.c b/convert.c
index 0d5e909..aa95834 100644
--- a/convert.c
+++ b/convert.c
@@ -110,7 +110,9 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
 			return 0;
 	}
 
-	strbuf_grow(buf, len);
+	/* only grow if not in place */
+	if (strbuf_avail(buf) + buf->len < len)
+		strbuf_grow(buf, len - buf->len);
 	dst = buf->buf;
 	if (action == CRLF_GUESS) {
 		/*
@@ -281,20 +283,19 @@ static int apply_filter(const char *path, const char *src, size_t len,
 		ret = 0;
 	}
 	if (close(pipe_feed[0])) {
-		ret = error("read from external filter %s failed", cmd);
+		error("read from external filter %s failed", cmd);
 		ret = 0;
 	}
 	status = finish_command(&child_process);
 	if (status) {
-		ret = error("external filter %s failed %d", cmd, -status);
+		error("external filter %s failed %d", cmd, -status);
 		ret = 0;
 	}
 
 	if (ret) {
-		*dst = nbuf;
-	} else {
-		strbuf_release(&nbuf);
+		strbuf_swap(dst, &nbuf);
 	}
+	strbuf_release(&nbuf);
 	return ret;
 }
 
@@ -422,7 +423,9 @@ static int ident_to_git(const char *path, const char *src, size_t len,
 	if (!ident || !count_ident(src, len))
 		return 0;
 
-	strbuf_grow(buf, len);
+	/* only grow if not in place */
+	if (strbuf_avail(buf) + buf->len < len)
+		strbuf_grow(buf, len - buf->len);
 	dst = buf->buf;
 	for (;;) {
 		dollar = memchr(src, '$', len);
-- 
1.5.3.4.207.gb504-dirty
Previous: Pierre HabouzitNext: Johannes Sixt
Message 2 of 18 in “Fix in-place editing in crlf_to_git and ident_to_git.”
  1. Fix in-place editing in crlf_to_git and ident_to_git.Pierre Habouzit, Oct 5, 2007
  2. Fix in-place editing functions in convert.cPierre Habouzit, Oct 5, 2007
  3. Johannes SixtOct 5, 2007
  4. Bernt HansenOct 5, 2007
  5. Linus TorvaldsOct 5, 2007
  6. Pierre HabouzitOct 5, 2007
  7. Sam RavnborgOct 5, 2007
  8. Pierre HabouzitOct 5, 2007
  9. Sam RavnborgOct 5, 2007
  10. Linus TorvaldsOct 5, 2007
  11. Sam RavnborgOct 5, 2007
  12. Linus TorvaldsOct 5, 2007
  13. Dmitry PotapovOct 5, 2007
  14. Linus TorvaldsOct 5, 2007
  15. Fix memory leak in apply_filter.Pierre Habouzit, Oct 5, 2007
  16. Fix memory leak in apply_filter.Pierre Habouzit, Oct 5, 2007
  17. Johannes SchindelinOct 5, 2007
  18. Pierre HabouzitOct 5, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.