Subject: [PATCH 09/19] tree-diff.c: reserve space in "base" for pathname concatenation

From: Nguyễn Thái Ngọc Duy <[email-masked]>
Date: Mon, 13 Dec 2010 16:46:46 +0700

This patch make sure that "base" parameter is writable. The callees
are free to modify it as long as base remains the same before
entering and after leaving the callee.

This avoids quite a bit of malloc and memcpy().

Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
---
 tree-diff.c |   87 +++++++++++++++++++++++++++-------------------------------
 1 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/tree-diff.c b/tree-diff.c
index 50d7e6d..a870f6c 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -6,34 +6,17 @@
 #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 void show_entry(struct diff_options *opt, const char *prefix,
+		       struct tree_desc *desc, char *base, int baselen);
 
-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 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,
+			      char *base, int baselen,
+			      struct diff_options *opt)
 {
 	unsigned mode1, mode2;
 	const char *path1, *path2;
 	const unsigned char *sha1, *sha2;
 	int cmp, pathlen1, pathlen2;
-	char *fullname;
 
 	sha1 = tree_entry_extract(t1, &path1, &mode1);
 	sha2 = tree_entry_extract(t2, &path2, &mode2);
@@ -64,26 +47,31 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
 
 	if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode1)) {
 		int retval;
-		char *newbase = malloc_base(base, baselen, path1, pathlen1);
+
+		memcpy(base + baselen, path1, pathlen1);
+		memcpy(base + baselen + pathlen1, "/", 2);
+
 		if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) {
-			newbase[baselen + pathlen1] = 0;
+			base[baselen + pathlen1] = 0;
 			opt->change(opt, mode1, mode2,
-				    sha1, sha2, newbase, 0, 0);
-			newbase[baselen + pathlen1] = '/';
+				    sha1, sha2, base, 0, 0);
+			base[baselen + pathlen1] = '/';
 		}
-		retval = diff_tree_sha1(sha1, sha2, newbase, opt);
-		free(newbase);
+		retval = diff_tree_sha1(sha1, sha2, base, opt);
+		base[baselen] = 0;
 		return retval;
 	}
 
-	fullname = malloc_fullname(base, baselen, path1, pathlen1);
-	opt->change(opt, mode1, mode2, sha1, sha2, fullname, 0, 0);
-	free(fullname);
+	memcpy(base + baselen, path1, pathlen1);
+	base[baselen + pathlen1] = 0;
+	opt->change(opt, mode1, mode2, sha1, sha2, base, 0, 0);
+	base[baselen] = 0;
 	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, char *base, int baselen)
 {
 	int all_interesting = 0;
 	while (desc->size) {
@@ -105,8 +93,8 @@ static void show_tree(struct diff_options *opt, const char *prefix, struct tree_
 }
 
 /* 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, char *base, int baselen)
 {
 	unsigned mode;
 	const char *path;
@@ -115,34 +103,38 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
 
 	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;
 
+		memcpy(base + baselen, path, pathlen);
+		memcpy(base + baselen + pathlen, "/", 2);
+
 		tree = read_sha1_file(sha1, &type, &size);
 		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] = '/';
+			base[baselen + pathlen] = 0;
+			opt->add_remove(opt, *prefix, mode, sha1, base, 0);
+			base[baselen + pathlen] = '/';
 		}
 
 		init_tree_desc(&inner, tree, size);
-		show_tree(opt, prefix, &inner, newbase, baselen + 1 + pathlen);
+		show_tree(opt, prefix, &inner, base, baselen + 1 + pathlen);
 
+		base[baselen] = 0;
 		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);
+		memcpy(base + baselen, path, pathlen);
+		base[baselen + pathlen] = 0;
+		opt->add_remove(opt, prefix[0], mode, sha1, base, 0);
+		base[baselen] = 0;
 	}
 }
 
-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, char *base,
+			       int baselen, struct diff_options *opt)
 {
 	int all_interesting = 0;
 	while (t->size) {
@@ -166,10 +158,13 @@ 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_, struct diff_options *opt)
 {
-	int baselen = strlen(base);
+	char base[PATH_MAX];
+	int baselen = strlen(base_);
 
+	memcpy(base, base_, baselen+1);
 	for (;;) {
 		if (DIFF_OPT_TST(opt, QUICK) &&
 		    DIFF_OPT_TST(opt, HAS_CHANGES))
-- 
1.7.3.3.476.g10a82