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

[PATCH 07/21] diff-tree: convert base+baselen to writable strbuf

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Dec 15, 2010, 15:02 UTC
Message-ID
<1292425376-14550-8-git-send-email-pclouds@gmail.com>
In-Reply-To
<1292425376-14550-1-git-send-email-pclouds@gmail.com>

In traversing trees, a full path is splitted into two parts: base directory and entry. They are however quite often concatenated whenever a full path is needed. Current code allocates a new buffer, do two memcpy(), use it, then release.

Instead this patch turns "base" to a writable, extendable buffer. When a concatenation is needed, the callee only needs to append "entry" to base, use it, then truncate the entry out again. "base" must remain unchanged before and after entering a function.

This avoids quite a bit of malloc() and memcpy().
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 tree-diff.c |  119 ++++++++++++++++++++++++++--------------------------------
 tree-walk.c |    5 +-
 tree-walk.h |    2 +-
 3 files changed, 57 insertions(+), 69 deletions(-)
diff --git a/tree-diff.c b/tree-diff.c
index 28a69dc..67fa6c4 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -6,34 +6,18 @@
 #include "diffcore.h"
 #include "tree.h"
 
-static char *malloc_base(const char *base, int baselen, const char *path, int pathlen)
-{
-	char *newbase = xmalloc(baselen + pathlen + 2);
-	memcpy(newbase, base, baselen);
-	memcpy(newbase + baselen, path, pathlen);
-	memcpy(newbase + baselen + pathlen, "/", 2);
-	return newbase;
-}
-
-static char *malloc_fullname(const char *base, int baselen, const char *path, int pathlen)
-{
-	char *fullname = xmalloc(baselen + pathlen + 1);
-	memcpy(fullname, base, baselen);
-	memcpy(fullname + baselen, path, pathlen);
-	fullname[baselen + pathlen] = 0;
-	return fullname;
-}
-
-static void show_entry(struct diff_options *opt, const char *prefix, struct tree_desc *desc,
-		       const char *base, int baselen);
+static void show_entry(struct diff_options *opt, const char *prefix,
+		       struct tree_desc *desc, struct strbuf *base);
 
-static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const char *base, int baselen, struct diff_options *opt)
+static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,
+			      struct strbuf *base, struct diff_options *opt)
 {
 	unsigned mode1, mode2;
 	const char *path1, *path2;
 	const unsigned char *sha1, *sha2;
 	int cmp, pathlen1, pathlen2;
-	char *fullname;
+	int old_baselen = base->len;
+	int retval = 0;
 
 	sha1 = tree_entry_extract(t1, &path1, &mode1);
 	sha2 = tree_entry_extract(t2, &path2, &mode2);
@@ -42,11 +26,11 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
 	pathlen2 = tree_entry_len(path2, sha2);
 	cmp = base_name_compare(path1, pathlen1, mode1, path2, pathlen2, mode2);
 	if (cmp < 0) {
-		show_entry(opt, "-", t1, base, baselen);
+		show_entry(opt, "-", t1, base);
 		return -1;
 	}
 	if (cmp > 0) {
-		show_entry(opt, "+", t2, base, baselen);
+		show_entry(opt, "+", t2, base);
 		return 1;
 	}
 	if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) && !hashcmp(sha1, sha2) && mode1 == mode2)
@@ -57,33 +41,29 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
 	 * file, we need to consider it a remove and an add.
 	 */
 	if (S_ISDIR(mode1) != S_ISDIR(mode2)) {
-		show_entry(opt, "-", t1, base, baselen);
-		show_entry(opt, "+", t2, base, baselen);
+		show_entry(opt, "-", t1, base);
+		show_entry(opt, "+", t2, base);
 		return 0;
 	}
 
+	strbuf_add(base, path1, pathlen1);
 	if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode1)) {
-		int retval;
-		char *newbase = malloc_base(base, baselen, path1, pathlen1);
 		if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) {
-			newbase[baselen + pathlen1] = 0;
 			opt->change(opt, mode1, mode2,
-				    sha1, sha2, newbase, 0, 0);
-			newbase[baselen + pathlen1] = '/';
+				    sha1, sha2, base->buf, 0, 0);
 		}
-		retval = diff_tree_sha1(sha1, sha2, newbase, opt);
-		free(newbase);
-		return retval;
+		strbuf_addch(base, '/');
+		retval = diff_tree_sha1(sha1, sha2, base->buf, opt);
 	}
-
-	fullname = malloc_fullname(base, baselen, path1, pathlen1);
-	opt->change(opt, mode1, mode2, sha1, sha2, fullname, 0, 0);
-	free(fullname);
+	else
+		opt->change(opt, mode1, mode2, sha1, sha2, base->buf, 0, 0);
+	strbuf_setlen(base, old_baselen);
 	return 0;
 }
 
 /* A whole sub-tree went away or appeared */
-static void show_tree(struct diff_options *opt, const char *prefix, struct tree_desc *desc, const char *base, int baselen)
+static void show_tree(struct diff_options *opt, const char *prefix,
+		      struct tree_desc *desc, struct strbuf *base)
 {
 	int all_interesting = 0;
 	while (desc->size) {
@@ -92,30 +72,32 @@ static void show_tree(struct diff_options *opt, const char *prefix, struct tree_
 		if (all_interesting)
 			show = 1;
 		else {
-			show = tree_entry_interesting(&desc->entry, base, baselen, &opt->pathspec);
+			show = tree_entry_interesting(&desc->entry, base,
+						      &opt->pathspec);
 			if (show == 2)
 				all_interesting = 1;
 		}
 		if (show < 0)
 			break;
 		if (show)
-			show_entry(opt, prefix, desc, base, baselen);
+			show_entry(opt, prefix, desc, base);
 		update_tree_entry(desc);
 	}
 }
 
 /* A file entry went away or appeared */
-static void show_entry(struct diff_options *opt, const char *prefix, struct tree_desc *desc,
-		       const char *base, int baselen)
+static void show_entry(struct diff_options *opt, const char *prefix,
+		       struct tree_desc *desc, struct strbuf *base)
 {
 	unsigned mode;
 	const char *path;
 	const unsigned char *sha1 = tree_entry_extract(desc, &path, &mode);
 	int pathlen = tree_entry_len(path, sha1);
+	int old_baselen = base->len;
 
+	strbuf_add(base, path, pathlen);
 	if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode)) {
 		enum object_type type;
-		char *newbase = malloc_base(base, baselen, path, pathlen);
 		struct tree_desc inner;
 		void *tree;
 		unsigned long size;
@@ -124,25 +106,22 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
 		if (!tree || type != OBJ_TREE)
 			die("corrupt tree sha %s", sha1_to_hex(sha1));
 
-		if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) {
-			newbase[baselen + pathlen] = 0;
-			opt->add_remove(opt, *prefix, mode, sha1, newbase, 0);
-			newbase[baselen + pathlen] = '/';
-		}
+		if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE))
+			opt->add_remove(opt, *prefix, mode, sha1, base->buf, 0);
 
-		init_tree_desc(&inner, tree, size);
-		show_tree(opt, prefix, &inner, newbase, baselen + 1 + pathlen);
+		strbuf_addch(base, '/');
 
+		init_tree_desc(&inner, tree, size);
+		show_tree(opt, prefix, &inner, base);
 		free(tree);
-		free(newbase);
-	} else {
-		char *fullname = malloc_fullname(base, baselen, path, pathlen);
-		opt->add_remove(opt, prefix[0], mode, sha1, fullname, 0);
-		free(fullname);
-	}
+	} else
+		opt->add_remove(opt, prefix[0], mode, sha1, base->buf, 0);
+
+	strbuf_setlen(base, old_baselen);
 }
 
-static void skip_uninteresting(struct tree_desc *t, const char *base, int baselen, struct diff_options *opt)
+static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,
+			       struct diff_options *opt)
 {
 	int all_interesting = 0;
 	while (t->size) {
@@ -151,7 +130,8 @@ static void skip_uninteresting(struct tree_desc *t, const char *base, int basele
 		if (all_interesting)
 			show = 1;
 		else {
-			show = tree_entry_interesting(&t->entry, base, baselen, &opt->pathspec);
+			show = tree_entry_interesting(&t->entry, base,
+						      &opt->pathspec);
 			if (show == 2)
 				all_interesting = 1;
 		}
@@ -166,31 +146,36 @@ static void skip_uninteresting(struct tree_desc *t, const char *base, int basele
 	}
 }
 
-int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, struct diff_options *opt)
+int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
+	      const char *base_str, struct diff_options *opt)
 {
-	int baselen = strlen(base);
+	struct strbuf base;
+	int baselen = strlen(base_str);
+
+	strbuf_init(&base, PATH_MAX);
+	strbuf_add(&base, base_str, baselen);
 
 	for (;;) {
 		if (DIFF_OPT_TST(opt, QUICK) &&
 		    DIFF_OPT_TST(opt, HAS_CHANGES))
 			break;
 		if (opt->pathspec.nr) {
-			skip_uninteresting(t1, base, baselen, opt);
-			skip_uninteresting(t2, base, baselen, opt);
+			skip_uninteresting(t1, &base, opt);
+			skip_uninteresting(t2, &base, opt);
 		}
 		if (!t1->size) {
 			if (!t2->size)
 				break;
-			show_entry(opt, "+", t2, base, baselen);
+			show_entry(opt, "+", t2, &base);
 			update_tree_entry(t2);
 			continue;
 		}
 		if (!t2->size) {
-			show_entry(opt, "-", t1, base, baselen);
+			show_entry(opt, "-", t1, &base);
 			update_tree_entry(t1);
 			continue;
 		}
-		switch (compare_tree_entry(t1, t2, base, baselen, opt)) {
+		switch (compare_tree_entry(t1, t2, &base, opt)) {
 		case -1:
 			update_tree_entry(t1);
 			continue;
@@ -203,6 +188,8 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, stru
 		}
 		die("git diff-tree: internal error");
 	}
+
+	strbuf_release(&base);
 	return 0;
 }
 
diff --git a/tree-walk.c b/tree-walk.c
index a2e2a99..0830676 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -466,12 +466,13 @@ int get_tree_entry(const unsigned char *tree_sha1, const char *name, unsigned ch
  *  - negative for "no, and no subsequent entries will be either"
  */
 int tree_entry_interesting(const struct name_entry *entry,
-			   const char *base, int baselen,
+			   const struct strbuf *base_buf,
 			   const struct pathspec *ps)
 {
 	int i;
-	int pathlen;
+	int pathlen, baselen = base_buf->len;
 	int never_interesting = -1;
+	const char *base = base_buf->buf;
 
 	if (!ps || !ps->nr)
 		return 1;
diff --git a/tree-walk.h b/tree-walk.h
index c12f0a2..f81c232 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -60,6 +60,6 @@ static inline int traverse_path_len(const struct traverse_info *info, const stru
 	return info->pathlen + tree_entry_len(n->path, n->sha1);
 }
 
-extern int tree_entry_interesting(const struct name_entry *, const char *, int, const struct pathspec *ps);
+extern int tree_entry_interesting(const struct name_entry *, const struct strbuf *, const struct pathspec *ps);
 
 #endif

-- 
1.7.3.3.476.g10a82
Previous: Nguyễn Thái Ngọc DuyNext: Nguyễn Thái Ngọc Duy
Message 8 of 44 in “nd/struct-pathspec v2”
  1. 00/21 nd/struct-pathspec v2Nguyễn Thái Ngọc Duy, Dec 15, 2010
  2. 01/21 Add struct pathspecNguyễn Thái Ngọc Duy, Dec 15, 2010
  3. 02/21 diff-no-index: use diff_tree_setup_paths()Nguyễn Thái Ngọc Duy, Dec 15, 2010
  4. 03/21 Convert struct diff_options to use struct pathspecNguyễn Thái Ngọc Duy, Dec 15, 2010
  5. 04/21 tree_entry_interesting(): remove dependency on struct diff_optionsNguyễn Thái Ngọc Duy, Dec 15, 2010
  6. 05/21 Move tree_entry_interesting() to tree-walk.c and export itNguyễn Thái Ngọc Duy, Dec 15, 2010
  7. 06/21 glossary: define pathspecNguyễn Thái Ngọc Duy, Dec 15, 2010
  8. 07/21 diff-tree: convert base+baselen to writable strbufNguyễn Thái Ngọc Duy, Dec 15, 2010
  9. 08/21 tree_entry_interesting(): refactor into separate smaller functionsNguyễn Thái Ngọc Duy, Dec 15, 2010
  10. 09/21 tree_entry_interesting(): support depth limitNguyễn Thái Ngọc Duy, Dec 15, 2010
  11. Junio C HamanoJan 28, 2011
  12. Nguyen Thai Ngoc DuyJan 29, 2011
  13. tree_entry_interesting(): with no pathspecs, everything will matchJunio C Hamano, Jan 31, 2011
  14. 10/21 tree_entry_interesting(): fix depth limit with overlapping pathspecsNguyễn Thái Ngọc Duy, Dec 15, 2010
  15. Junio C HamanoDec 16, 2010
  16. Nguyen Thai Ngoc DuyDec 17, 2010
  17. Junio C HamanoDec 17, 2010
  18. Nguyen Thai Ngoc DuyDec 18, 2010
  19. 11/21 tree_entry_interesting(): support wildcard matchingNguyễn Thái Ngọc Duy, Dec 15, 2010
  20. diff-tree doc: correct & remove wrong documentationÆvar Arnfjörð Bjarmason, Feb 4, 2019
  21. Duy NguyenFeb 4, 2019
  22. Junio C HamanoFeb 4, 2019
  23. Ævar Arnfjörð BjarmasonFeb 4, 2019
  24. 12/21 tree_entry_interesting(): optimize wildcard matching when base is matchedNguyễn Thái Ngọc Duy, Dec 15, 2010
  25. 13/21 pathspec: add match_pathspec_depth()Nguyễn Thái Ngọc Duy, Dec 15, 2010
  26. 14/21 Convert ce_path_match() to use struct pathspecNguyễn Thái Ngọc Duy, Dec 15, 2010
  27. Junio C HamanoDec 17, 2010
  28. Nguyen Thai Ngoc DuyDec 17, 2010
  29. 14/21 struct rev_info: convert prune_data to struct pathspecNguyễn Thái Ngọc Duy, Dec 17, 2010
  30. 15/21 Convert ce_path_match() to use struct pathspecNguyễn Thái Ngọc Duy, Dec 17, 2010
  31. Junio C HamanoDec 17, 2010
  32. Nguyen Thai Ngoc DuyDec 17, 2010
  33. Junio C HamanoDec 17, 2010
  34. 15/21 Convert ce_path_match() to use match_pathspec_depth()Nguyễn Thái Ngọc Duy, Dec 15, 2010
  35. 16/21 grep: convert to use struct pathspecNguyễn Thái Ngọc Duy, Dec 15, 2010
  36. 17/21 grep: use match_pathspec_depth() for cache/worktree greppingNguyễn Thái Ngọc Duy, Dec 15, 2010
  37. 18/21 strbuf: allow "buf" to point to the middle of the allocated bufferNguyễn Thái Ngọc Duy, Dec 15, 2010
  38. 19/21 grep: use writable strbuf from caller in grep_tree()Nguyễn Thái Ngọc Duy, Dec 15, 2010
  39. Junio C HamanoDec 17, 2010
  40. Nguyen Thai Ngoc DuyDec 17, 2010
  41. 19/21 grep: use writable strbuf from caller for grep_tree()Nguyễn Thái Ngọc Duy, Dec 17, 2010
  42. 20/21 grep: drop pathspec_matches() in favor of tree_entry_interesting()Nguyễn Thái Ngọc Duy, Dec 15, 2010
  43. 20/21 grep: drop pathspec_matches() in favor of tree_entry_interesting()Nguyễn Thái Ngọc Duy, Dec 17, 2010
  44. 21/21 t7810: overlapping pathspecs and depth limitNguyễn Thái Ngọc Duy, Dec 15, 2010

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.