threads / patch / 4323

patch, 4 partsStart migrating away from the "struct tree_entry" list

Subject: [PATCH 0/4] Start migrating away from the "struct tree_entry" list

## tl;dr

5 messages between May 28, 2006 and May 28, 2006. Diffs are folded; open one to read it.

replies: 4people: 1as markdown or json

Linus Torvalds· May 28, 2006, 22:03 UTC · lore

The old "tree.h" parsing logic (before I did the "tree-walk" tree buffer parser) would generate a list of "struct tree_entry" objects, which was basically a simpler-to-walk version of the raw tree buffer.

Now, the problem with that simpler-to-walk version is that it's a lot less efficient than just walking the raw tree buffer in-place, and a lot of the core git tree walkers already use the much more efficient in-place tree-walker interfaces. Which means that we have _two_ independent ways of walking the tree. Very annoying.

This series of four patches (on top of my trivial memory leak fix for git rev-list) gets rid of the tree-entry list from tree parsing, and converts most users over to use the tree-walker, and has a small compatibility helper function to keep some users that are hard to convert using what is now obviously just a temporary list.

Apart from being one more step in the direction of cleaning up tree walking, it also makes things more efficient, by not needlessly creating that silly list that most users don't actually want. I used the same test vector as I did for the git rev-list leaker: doing a

	git-rev-list --all --objects

on the historical kernel archive (that one has 566k objects and is more than twice the size of the current kernel git tree). The series brings that down from

	74.67user 0.45system 1:15.43elapsed
to
	62.50user 0.41system 1:02.93elapsed
for me.

NOTE! I think the git-rev-list leak fix can go into the main tree immediately, but this series should probably be kept in "next" for a while, just to verify that it all is good..

		Linus
Linus Torvalds· May 28, 2006, 22:07 UTC · re: Linus Torvalds · lore

[PATCH 1/4] Add raw tree buffer info to "struct tree"

This allows us to avoid allocating information for names etc, because we can just use the information from the tree buffer directly.

We still keep the old "tree_entry_list" in struct tree as well, so old users aren't affected, apart from the fact that the allocations are different (if you free a tree entry, you should no longer free the name allocation for it, since it's allocated as part of "tree->buffer")

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
Show changes to 6 files +36 −34

builtin-read-tree.c, builtin-rev-list.c, fsck-objects.c, object.c, tree.c, tree.h

diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index ec40d01..740a8c7 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -38,7 +38,7 @@ static struct tree_entry_list df_conflic
 
 typedef int (*merge_fn_t)(struct cache_entry **src);
 
-static int entcmp(char *name1, int dir1, char *name2, int dir2)
+static int entcmp(const char *name1, int dir1, const char *name2, int dir2)
 {
 	int len1 = strlen(name1);
 	int len2 = strlen(name2);
@@ -66,7 +66,7 @@ static int unpack_trees_rec(struct tree_
 	int src_size = len + 1;
 	do {
 		int i;
-		char *first;
+		const char *first;
 		int firstdir = 0;
 		int pathlen;
 		unsigned ce_size;
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 5277d3c..72c1549 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -136,10 +136,11 @@ static struct object_list **process_tree
 			p = process_tree(entry->item.tree, p, &me, entry->name);
 		else
 			p = process_blob(entry->item.blob, p, &me, entry->name);
-		free(entry->name);
 		free(entry);
 		entry = next;
 	}
+	free(tree->buffer);
+	tree->buffer = NULL;
 	return p;
 }
 
diff --git a/fsck-objects.c b/fsck-objects.c
index 59b2590..a0290b0 100644
--- a/fsck-objects.c
+++ b/fsck-objects.c
@@ -197,17 +197,16 @@ static int fsck_tree(struct tree *item)
 			default:
 				break;
 			}
-			free(last->name);
 			free(last);
 		}
 
 		last = entry;
 	}
-	if (last) {
-		free(last->name);
+	if (last)
 		free(last);
-	}
 	item->entries = NULL;
+	free(item->buffer);
+	item->buffer = NULL;
 
 	retval = 0;
 	if (has_full_path) {
diff --git a/object.c b/object.c
index 4d46e0d..1a7823c 100644
--- a/object.c
+++ b/object.c
@@ -200,8 +200,11 @@ struct object *parse_object(const unsign
 			obj = &blob->object;
 		} else if (!strcmp(type, tree_type)) {
 			struct tree *tree = lookup_tree(sha1);
-			parse_tree_buffer(tree, buffer, size);
 			obj = &tree->object;
+			if (!tree->object.parsed) {
+				parse_tree_buffer(tree, buffer, size);
+				buffer = NULL;
+			}
 		} else if (!strcmp(type, commit_type)) {
 			struct commit *commit = lookup_commit(sha1);
 			parse_commit_buffer(commit, buffer, size);
diff --git a/tree.c b/tree.c
index d599fb5..1e76d9c 100644
--- a/tree.c
+++ b/tree.c
@@ -3,6 +3,7 @@ #include "tree.h"
 #include "blob.h"
 #include "commit.h"
 #include "tag.h"
+#include "tree-walk.h"
 #include <stdlib.h>
 
 const char *tree_type = "tree";
@@ -145,46 +146,45 @@ struct tree *lookup_tree(const unsigned 
 
 int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size)
 {
-	void *bufptr = buffer;
+	struct tree_desc desc;
 	struct tree_entry_list **list_p;
 	int n_refs = 0;
 
 	if (item->object.parsed)
 		return 0;
 	item->object.parsed = 1;
+	item->buffer = buffer;
+	item->size = size;
+
+	desc.buf = buffer;
+	desc.size = size;
+
 	list_p = &item->entries;
-	while (size) {
-		struct object *obj;
+	while (desc.size) {
+		unsigned mode;
+		const char *path;
+		const unsigned char *sha1;
 		struct tree_entry_list *entry;
-		int len = 1+strlen(bufptr);
-		unsigned char *file_sha1 = bufptr + len;
-		char *path = strchr(bufptr, ' ');
-		unsigned int mode;
-		if (size < len + 20 || !path || 
-		    sscanf(bufptr, "%o", &mode) != 1)
-			return -1;
+
+		sha1 = tree_entry_extract(&desc, &path, &mode);
 
 		entry = xmalloc(sizeof(struct tree_entry_list));
-		entry->name = strdup(path + 1);
+		entry->name = path;
+		entry->mode = mode;
 		entry->directory = S_ISDIR(mode) != 0;
 		entry->executable = (mode & S_IXUSR) != 0;
 		entry->symlink = S_ISLNK(mode) != 0;
-		entry->zeropad = *(char *)bufptr == '0';
-		entry->mode = mode;
+		entry->zeropad = *(const char *)(desc.buf) == '0';
 		entry->next = NULL;
 
-		bufptr += len + 20;
-		size -= len + 20;
+		update_tree_entry(&desc);
 
 		if (entry->directory) {
-			entry->item.tree = lookup_tree(file_sha1);
-			obj = &entry->item.tree->object;
+			entry->item.tree = lookup_tree(sha1);
 		} else {
-			entry->item.blob = lookup_blob(file_sha1);
-			obj = &entry->item.blob->object;
+			entry->item.blob = lookup_blob(sha1);
 		}
-		if (obj)
-			n_refs++;
+		n_refs++;
 		*list_p = entry;
 		list_p = &entry->next;
 	}
@@ -206,7 +206,6 @@ int parse_tree(struct tree *item)
 	 char type[20];
 	 void *buffer;
 	 unsigned long size;
-	 int ret;
 
 	if (item->object.parsed)
 		return 0;
@@ -219,9 +218,7 @@ int parse_tree(struct tree *item)
 		return error("Object %s not a tree",
 			     sha1_to_hex(item->object.sha1));
 	}
-	ret = parse_tree_buffer(item, buffer, size);
-	free(buffer);
-	return ret;
+	return parse_tree_buffer(item, buffer, size);
 }
 
 struct tree *parse_tree_indirect(const unsigned char *sha1)
diff --git a/tree.h b/tree.h
index 330ab64..066ac5d 100644
--- a/tree.h
+++ b/tree.h
@@ -12,7 +12,7 @@ struct tree_entry_list {
 	unsigned symlink : 1;
 	unsigned zeropad : 1;
 	unsigned int mode;
-	char *name;
+	const char *name;
 	union {
 		struct object *any;
 		struct tree *tree;
@@ -22,6 +22,8 @@ struct tree_entry_list {
 
 struct tree {
 	struct object object;
+	void *buffer;
+	unsigned long size;
 	struct tree_entry_list *entries;
 };
Linus Torvalds· May 28, 2006, 22:10 UTC · re: Linus Torvalds · lore

[PATCH 2/4] Make "tree_entry" have a SHA1 instead of a union of object pointers

This is preparatory work for further cleanups, where we try to make tree_entry look more like the more efficient tree-walk descriptor.

Instead of having a union of pointers to blob/tree/objects, this just makes "struct tree_entry" have the raw SHA1, and makes all the users use that instead (often that implies adding a "lookup_tree(..)" on the sha1, but sometimes the user just wanted the SHA1 in the first place, and it just avoids an unnecessary indirection).

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
 blame.c             |    4 ++--
 builtin-read-tree.c |    7 ++++---
 builtin-rev-list.c  |    4 ++--
 fsck-objects.c      |    1 +
 http-push.c         |    4 ++--
 object.c            |    2 +-
 revision.c          |    4 ++--
 tree.c              |   25 ++++++++++++++-----------
 tree.h              |    8 ++------
 9 files changed, 30 insertions(+), 29 deletions(-)
Show changes to 9 files +30 −29

blame.c, builtin-read-tree.c, builtin-rev-list.c, fsck-objects.c, http-push.c, object.c, revision.c, tree.c, tree.h

diff --git a/blame.c b/blame.c
index 99ceea8..88bfec2 100644
--- a/blame.c
+++ b/blame.c
@@ -149,7 +149,7 @@ static void free_patch(struct patch *p)
 	free(p);
 }
 
-static int get_blob_sha1_internal(unsigned char *sha1, const char *base,
+static int get_blob_sha1_internal(const unsigned char *sha1, const char *base,
 				  int baselen, const char *pathname,
 				  unsigned mode, int stage);
 
@@ -178,7 +178,7 @@ static int get_blob_sha1(struct tree *t,
 	return 0;
 }
 
-static int get_blob_sha1_internal(unsigned char *sha1, const char *base,
+static int get_blob_sha1_internal(const unsigned char *sha1, const char *base,
 				  int baselen, const char *pathname,
 				  unsigned mode, int stage)
 {
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 740a8c7..f0b8dad 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -160,9 +160,10 @@ #endif
 			}
 
 			if (posns[i]->directory) {
+				struct tree *tree = lookup_tree(posns[i]->sha1);
 				any_dirs = 1;
-				parse_tree(posns[i]->item.tree);
-				subposns[i] = posns[i]->item.tree->entries;
+				parse_tree(tree);
+				subposns[i] = tree->entries;
 				posns[i] = posns[i]->next;
 				src[i + merge] = &df_conflict_entry;
 				continue;
@@ -186,7 +187,7 @@ #endif
 
 			any_files = 1;
 
-			memcpy(ce->sha1, posns[i]->item.any->sha1, 20);
+			memcpy(ce->sha1, posns[i]->sha1, 20);
 			src[i + merge] = ce;
 			subposns[i] = &df_conflict_list;
 			posns[i] = posns[i]->next;
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 72c1549..94f520b 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -133,9 +133,9 @@ static struct object_list **process_tree
 	while (entry) {
 		struct tree_entry_list *next = entry->next;
 		if (entry->directory)
-			p = process_tree(entry->item.tree, p, &me, entry->name);
+			p = process_tree(lookup_tree(entry->sha1), p, &me, entry->name);
 		else
-			p = process_blob(entry->item.blob, p, &me, entry->name);
+			p = process_blob(lookup_blob(entry->sha1), p, &me, entry->name);
 		free(entry);
 		entry = next;
 	}
diff --git a/fsck-objects.c b/fsck-objects.c
index a0290b0..44b6465 100644
--- a/fsck-objects.c
+++ b/fsck-objects.c
@@ -441,6 +441,7 @@ int main(int argc, char **argv)
 {
 	int i, heads;
 
+	track_object_refs = 1;
 	setup_git_directory();
 
 	for (i = 1; i < argc; i++) {
diff --git a/http-push.c b/http-push.c
index b4327d9..f492a5d 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1733,9 +1733,9 @@ static struct object_list **process_tree
 	while (entry) {
 		struct tree_entry_list *next = entry->next;
 		if (entry->directory)
-			p = process_tree(entry->item.tree, p, &me, entry->name);
+			p = process_tree(lookup_tree(entry->sha1), p, &me, entry->name);
 		else
-			p = process_blob(entry->item.blob, p, &me, entry->name);
+			p = process_blob(lookup_blob(entry->sha1), p, &me, entry->name);
 		free(entry);
 		entry = next;
 	}
diff --git a/object.c b/object.c
index 1a7823c..9adc874 100644
--- a/object.c
+++ b/object.c
@@ -9,7 +9,7 @@ struct object **objs;
 static int nr_objs;
 int obj_allocs;
 
-int track_object_refs = 1;
+int track_object_refs = 0;
 
 static int hashtable_index(const unsigned char *sha1)
 {
diff --git a/revision.c b/revision.c
index 2294b16..35f8e3b 100644
--- a/revision.c
+++ b/revision.c
@@ -68,9 +68,9 @@ void mark_tree_uninteresting(struct tree
 	while (entry) {
 		struct tree_entry_list *next = entry->next;
 		if (entry->directory)
-			mark_tree_uninteresting(entry->item.tree);
+			mark_tree_uninteresting(lookup_tree(entry->sha1));
 		else
-			mark_blob_uninteresting(entry->item.blob);
+			mark_blob_uninteresting(lookup_blob(entry->sha1));
 		free(entry);
 		entry = next;
 	}
diff --git a/tree.c b/tree.c
index 1e76d9c..ba8742c 100644
--- a/tree.c
+++ b/tree.c
@@ -8,7 +8,7 @@ #include <stdlib.h>
 
 const char *tree_type = "tree";
 
-static int read_one_entry(unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage)
+static int read_one_entry(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage)
 {
 	int len;
 	unsigned int size;
@@ -89,7 +89,7 @@ int read_tree_recursive(struct tree *tre
 				      current->mode, match))
 			continue;
 
-		switch (fn(current->item.any->sha1, base, baselen,
+		switch (fn(current->sha1, base, baselen,
 			   current->name, current->mode, stage)) {
 		case 0:
 			continue;
@@ -107,7 +107,7 @@ int read_tree_recursive(struct tree *tre
 			memcpy(newbase, base, baselen);
 			memcpy(newbase + baselen, current->name, pathlen);
 			newbase[baselen + pathlen] = '/';
-			retval = read_tree_recursive(current->item.tree,
+			retval = read_tree_recursive(lookup_tree(current->sha1),
 						     newbase,
 						     baselen + pathlen + 1,
 						     stage, match, fn);
@@ -170,6 +170,7 @@ int parse_tree_buffer(struct tree *item,
 
 		entry = xmalloc(sizeof(struct tree_entry_list));
 		entry->name = path;
+		entry->sha1 = sha1;
 		entry->mode = mode;
 		entry->directory = S_ISDIR(mode) != 0;
 		entry->executable = (mode & S_IXUSR) != 0;
@@ -178,12 +179,6 @@ int parse_tree_buffer(struct tree *item,
 		entry->next = NULL;
 
 		update_tree_entry(&desc);
-
-		if (entry->directory) {
-			entry->item.tree = lookup_tree(sha1);
-		} else {
-			entry->item.blob = lookup_blob(sha1);
-		}
 		n_refs++;
 		*list_p = entry;
 		list_p = &entry->next;
@@ -193,8 +188,16 @@ int parse_tree_buffer(struct tree *item,
 		struct tree_entry_list *entry;
 		unsigned i = 0;
 		struct object_refs *refs = alloc_object_refs(n_refs);
-		for (entry = item->entries; entry; entry = entry->next)
-			refs->ref[i++] = entry->item.any;
+		for (entry = item->entries; entry; entry = entry->next) {
+			struct object *obj;
+
+			if (entry->directory)
+				obj = &lookup_tree(entry->sha1)->object;
+			else
+				obj = &lookup_blob(entry->sha1)->object;
+			refs->ref[i++] = obj;
+		}
+				
 		set_object_refs(&item->object, refs);
 	}
 
diff --git a/tree.h b/tree.h
index 066ac5d..a27bae4 100644
--- a/tree.h
+++ b/tree.h
@@ -13,11 +13,7 @@ struct tree_entry_list {
 	unsigned zeropad : 1;
 	unsigned int mode;
 	const char *name;
-	union {
-		struct object *any;
-		struct tree *tree;
-		struct blob *blob;
-	} item;
+	const unsigned char *sha1;
 };
 
 struct tree {
@@ -37,7 +33,7 @@ int parse_tree(struct tree *tree);
 struct tree *parse_tree_indirect(const unsigned char *sha1);
 
 #define READ_TREE_RECURSIVE 1
-typedef int (*read_tree_fn_t)(unsigned char *, const char *, int, const char *, unsigned int, int);
+typedef int (*read_tree_fn_t)(const unsigned char *, const char *, int, const char *, unsigned int, int);
 
 extern int read_tree_recursive(struct tree *tree,
 			       const char *base, int baselen,
Linus Torvalds· May 28, 2006, 22:11 UTC · re: Linus Torvalds · lore

[PATCH 3/4] Switch "read_tree_recursive()" over to tree-walk functionality

Don't use the tree_entry list, it really had no major reason not to just walk the raw tree instead.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
 tree.c |   33 ++++++++++++++++++++-------------
 1 files changed, 20 insertions(+), 13 deletions(-)
Show changes to tree.c +20 −13
diff --git a/tree.c b/tree.c
index ba8742c..8a7fdd4 100644
--- a/tree.c
+++ b/tree.c
@@ -78,19 +78,26 @@ int read_tree_recursive(struct tree *tre
 			int stage, const char **match,
 			read_tree_fn_t fn)
 {
-	struct tree_entry_list *list;
+	struct tree_desc desc;
+
 	if (parse_tree(tree))
 		return -1;
-	list = tree->entries;
-	while (list) {
-		struct tree_entry_list *current = list;
-		list = list->next;
-		if (!match_tree_entry(base, baselen, current->name,
-				      current->mode, match))
+
+	desc.buf = tree->buffer;
+	desc.size = tree->size;
+
+	while (desc.size) {
+		unsigned mode;
+		const char *name;
+		const unsigned char *sha1;
+
+		sha1 = tree_entry_extract(&desc, &name, &mode);
+		update_tree_entry(&desc);
+
+		if (!match_tree_entry(base, baselen, name, mode, match))
 			continue;
 
-		switch (fn(current->sha1, base, baselen,
-			   current->name, current->mode, stage)) {
+		switch (fn(sha1, base, baselen, name, mode, stage)) {
 		case 0:
 			continue;
 		case READ_TREE_RECURSIVE:
@@ -98,16 +105,16 @@ int read_tree_recursive(struct tree *tre
 		default:
 			return -1;
 		}
-		if (current->directory) {
+		if (S_ISDIR(mode)) {
 			int retval;
-			int pathlen = strlen(current->name);
+			int pathlen = strlen(name);
 			char *newbase;
 
 			newbase = xmalloc(baselen + 1 + pathlen);
 			memcpy(newbase, base, baselen);
-			memcpy(newbase + baselen, current->name, pathlen);
+			memcpy(newbase + baselen, name, pathlen);
 			newbase[baselen + pathlen] = '/';
-			retval = read_tree_recursive(lookup_tree(current->sha1),
+			retval = read_tree_recursive(lookup_tree(sha1),
 						     newbase,
 						     baselen + pathlen + 1,
 						     stage, match, fn);
Linus Torvalds· May 28, 2006, 22:13 UTC · re: Linus Torvalds · lore

[PATCH 4/4] Remove "tree->entries" tree-entry list from tree parser

This finally removes the tree-entry list from "struct tree", since most of the users can just use the tree-walk infrastructure to walk the raw tree buffers instead of the tree-entry list.

The tree-entry list is inefficient, and generates tons of small allocations for no good reason. The tree-walk infrastructure is generally no harder to use than following a linked list, and allows us to do most tree parsing in-place.

Some programs still use the old tree-entry lists, and are a bit painful to convert without major surgery. For them we have a helper function that creates a temporary tree-entry list on demand. We can convert those too eventually, but with this they no longer affect any users who don't need the explicit lists.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
 builtin-ls-tree.c   |    2 +
 builtin-read-tree.c |    4 +--
 builtin-rev-list.c  |   26 ++++++++++------
 fetch.c             |   16 +++++++---
 fsck-objects.c      |    7 +++-
 http-push.c         |   30 +++++++++++++------
 revision.c          |    3 +-
 tree.c              |   81 ++++++++++++++++++++++++++++++++++++---------------
 tree.h              |    4 ++-
 9 files changed, 116 insertions(+), 57 deletions(-)
Show changes to 9 files +116 −57

builtin-ls-tree.c, builtin-read-tree.c, builtin-rev-list.c, fetch.c, fsck-objects.c, http-push.c, revision.c, tree.c, tree.h

diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
index 48385d5..b8d0d88 100644
--- a/builtin-ls-tree.c
+++ b/builtin-ls-tree.c
@@ -53,7 +53,7 @@ static int show_recursive(const char *ba
 	}
 }
 
-static int show_tree(unsigned char *sha1, const char *base, int baselen,
+static int show_tree(const unsigned char *sha1, const char *base, int baselen,
 		     const char *pathname, unsigned mode, int stage)
 {
 	int retval = 0;
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index f0b8dad..da0731c 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -163,7 +163,7 @@ #endif
 				struct tree *tree = lookup_tree(posns[i]->sha1);
 				any_dirs = 1;
 				parse_tree(tree);
-				subposns[i] = tree->entries;
+				subposns[i] = create_tree_entry_list(tree);
 				posns[i] = posns[i]->next;
 				src[i + merge] = &df_conflict_entry;
 				continue;
@@ -368,7 +368,7 @@ static int unpack_trees(merge_fn_t fn)
 	if (len) {
 		posns = xmalloc(len * sizeof(struct tree_entry_list *));
 		for (i = 0; i < len; i++) {
-			posns[i] = ((struct tree *) posn->item)->entries;
+			posns[i] = create_tree_entry_list((struct tree *) posn->item);
 			posn = posn->next;
 		}
 		if (unpack_trees_rec(posns, len, "", fn, &indpos))
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 94f520b..6e2b898 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -113,7 +113,7 @@ static struct object_list **process_tree
 					 const char *name)
 {
 	struct object *obj = &tree->object;
-	struct tree_entry_list *entry;
+	struct tree_desc desc;
 	struct name_path me;
 
 	if (!revs.tree_objects)
@@ -128,16 +128,22 @@ static struct object_list **process_tree
 	me.up = path;
 	me.elem = name;
 	me.elem_len = strlen(name);
-	entry = tree->entries;
-	tree->entries = NULL;
-	while (entry) {
-		struct tree_entry_list *next = entry->next;
-		if (entry->directory)
-			p = process_tree(lookup_tree(entry->sha1), p, &me, entry->name);
+
+	desc.buf = tree->buffer;
+	desc.size = tree->size;
+
+	while (desc.size) {
+		unsigned mode;
+		const char *name;
+		const unsigned char *sha1;
+
+		sha1 = tree_entry_extract(&desc, &name, &mode);
+		update_tree_entry(&desc);
+
+		if (S_ISDIR(mode))
+			p = process_tree(lookup_tree(sha1), p, &me, name);
 		else
-			p = process_blob(lookup_blob(entry->sha1), p, &me, entry->name);
-		free(entry);
-		entry = next;
+			p = process_blob(lookup_blob(sha1), p, &me, name);
 	}
 	free(tree->buffer);
 	tree->buffer = NULL;
diff --git a/fetch.c b/fetch.c
index f7f8902..d9fe41f 100644
--- a/fetch.c
+++ b/fetch.c
@@ -41,16 +41,22 @@ static int process_tree(struct tree *tre
 	if (parse_tree(tree))
 		return -1;
 
-	entry = tree->entries;
-	tree->entries = NULL;
+	entry = create_tree_entry_list(tree);
 	while (entry) {
 		struct tree_entry_list *next = entry->next;
-		if (process(entry->item.any))
-			return -1;
-		free(entry->name);
+
+		if (entry->directory) {
+			struct tree *tree = lookup_tree(entry->sha1);
+			process_tree(tree);
+		} else {
+			struct blob *blob = lookup_blob(entry->sha1);
+			process(&blob->object);
+		}
 		free(entry);
 		entry = next;
 	}
+	free(tree->buffer);
+	tree->buffer = NULL;
 	return 0;
 }
 
diff --git a/fsck-objects.c b/fsck-objects.c
index 44b6465..ec99a7a 100644
--- a/fsck-objects.c
+++ b/fsck-objects.c
@@ -10,6 +10,7 @@ #include "refs.h"
 #include "pack.h"
 
 #define REACHABLE 0x0001
+#define SEEN      0x0002
 
 static int show_root = 0;
 static int show_tags = 0;
@@ -160,7 +161,7 @@ static int fsck_tree(struct tree *item)
 	struct tree_entry_list *entry, *last;
 
 	last = NULL;
-	for (entry = item->entries; entry; entry = entry->next) {
+	for (entry = create_tree_entry_list(item); entry; entry = entry->next) {
 		if (strchr(entry->name, '/'))
 			has_full_path = 1;
 		has_zero_pad |= entry->zeropad;
@@ -204,7 +205,6 @@ static int fsck_tree(struct tree *item)
 	}
 	if (last)
 		free(last);
-	item->entries = NULL;
 	free(item->buffer);
 	item->buffer = NULL;
 
@@ -276,6 +276,9 @@ static int fsck_sha1(unsigned char *sha1
 	struct object *obj = parse_object(sha1);
 	if (!obj)
 		return error("%s: object not found", sha1_to_hex(sha1));
+	if (obj->flags & SEEN)
+		return 0;
+	obj->flags |= SEEN;
 	if (obj->type == blob_type)
 		return 0;
 	if (obj->type == tree_type)
diff --git a/http-push.c b/http-push.c
index f492a5d..72ad89c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1704,6 +1704,7 @@ static struct object_list **process_blob
 		return p;
 
 	obj->flags |= SEEN;
+	name = strdup(name);
 	return add_object(obj, p, path, name);
 }
 
@@ -1713,7 +1714,7 @@ static struct object_list **process_tree
 					 const char *name)
 {
 	struct object *obj = &tree->object;
-	struct tree_entry_list *entry;
+	struct tree_desc desc;
 	struct name_path me;
 
 	obj->flags |= LOCAL;
@@ -1724,21 +1725,30 @@ static struct object_list **process_tree
 		die("bad tree object %s", sha1_to_hex(obj->sha1));
 
 	obj->flags |= SEEN;
+	name = strdup(name);
 	p = add_object(obj, p, NULL, name);
 	me.up = path;
 	me.elem = name;
 	me.elem_len = strlen(name);
-	entry = tree->entries;
-	tree->entries = NULL;
-	while (entry) {
-		struct tree_entry_list *next = entry->next;
-		if (entry->directory)
-			p = process_tree(lookup_tree(entry->sha1), p, &me, entry->name);
+
+	desc.buf = tree->buffer;
+	desc.size = tree->size;
+
+	while (desc.size) {
+		unsigned mode;
+		const char *name;
+		const unsigned char *sha1;
+
+		sha1 = tree_entry_extract(&desc, &name, &mode);
+		update_tree_entry(&desc);
+
+		if (S_ISDIR(mode))
+			p = process_tree(lookup_tree(sha1), p, &me, name);
 		else
-			p = process_blob(lookup_blob(entry->sha1), p, &me, entry->name);
-		free(entry);
-		entry = next;
+			p = process_blob(lookup_blob(sha1), p, &me, name);
 	}
+	free(tree->buffer);
+	tree->buffer = NULL;
 	return p;
 }
 
diff --git a/revision.c b/revision.c
index 35f8e3b..c366178 100644
--- a/revision.c
+++ b/revision.c
@@ -63,8 +63,7 @@ void mark_tree_uninteresting(struct tree
 		return;
 	if (parse_tree(tree) < 0)
 		die("bad tree %s", sha1_to_hex(obj->sha1));
-	entry = tree->entries;
-	tree->entries = NULL;
+	entry = create_tree_entry_list(tree);
 	while (entry) {
 		struct tree_entry_list *next = entry->next;
 		if (entry->directory)
diff --git a/tree.c b/tree.c
index 8a7fdd4..db6e59f 100644
--- a/tree.c
+++ b/tree.c
@@ -151,22 +151,65 @@ struct tree *lookup_tree(const unsigned 
 	return (struct tree *) obj;
 }
 
-int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size)
+static int track_tree_refs(struct tree *item)
 {
+	int n_refs = 0, i;
+	struct object_refs *refs;
 	struct tree_desc desc;
-	struct tree_entry_list **list_p;
-	int n_refs = 0;
 
+	/* Count how many entries there are.. */
+	desc.buf = item->buffer;
+	desc.size = item->size;
+	while (desc.size) {
+		n_refs++;
+		update_tree_entry(&desc);
+	}
+
+	/* Allocate object refs and walk it again.. */
+	i = 0;
+	refs = alloc_object_refs(n_refs);
+	desc.buf = item->buffer;
+	desc.size = item->size;
+	while (desc.size) {
+		unsigned mode;
+		const char *name;
+		const unsigned char *sha1;
+		struct object *obj;
+
+		sha1 = tree_entry_extract(&desc, &name, &mode);
+		update_tree_entry(&desc);
+		if (S_ISDIR(mode))
+			obj = &lookup_tree(sha1)->object;
+		else
+			obj = &lookup_blob(sha1)->object;
+		refs->ref[i++] = obj;
+	}
+	set_object_refs(&item->object, refs);
+	return 0;
+}
+
+int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size)
+{
 	if (item->object.parsed)
 		return 0;
 	item->object.parsed = 1;
 	item->buffer = buffer;
 	item->size = size;
 
-	desc.buf = buffer;
-	desc.size = size;
+	if (track_object_refs)
+		track_tree_refs(item);
+	return 0;
+}
+
+struct tree_entry_list *create_tree_entry_list(struct tree *tree)
+{
+	struct tree_desc desc;
+	struct tree_entry_list *ret = NULL;
+	struct tree_entry_list **list_p = &ret;
+
+	desc.buf = tree->buffer;
+	desc.size = tree->size;
 
-	list_p = &item->entries;
 	while (desc.size) {
 		unsigned mode;
 		const char *path;
@@ -186,29 +229,19 @@ int parse_tree_buffer(struct tree *item,
 		entry->next = NULL;
 
 		update_tree_entry(&desc);
-		n_refs++;
 		*list_p = entry;
 		list_p = &entry->next;
 	}
+	return ret;
+}
 
-	if (track_object_refs) {
-		struct tree_entry_list *entry;
-		unsigned i = 0;
-		struct object_refs *refs = alloc_object_refs(n_refs);
-		for (entry = item->entries; entry; entry = entry->next) {
-			struct object *obj;
-
-			if (entry->directory)
-				obj = &lookup_tree(entry->sha1)->object;
-			else
-				obj = &lookup_blob(entry->sha1)->object;
-			refs->ref[i++] = obj;
-		}
-				
-		set_object_refs(&item->object, refs);
+void free_tree_entry_list(struct tree_entry_list *list)
+{
+	while (list) {
+		struct tree_entry_list *next = list->next;
+		free(list);
+		list = next;
 	}
-
-	return 0;
 }
 
 int parse_tree(struct tree *item)
diff --git a/tree.h b/tree.h
index a27bae4..c7b5248 100644
--- a/tree.h
+++ b/tree.h
@@ -20,9 +20,11 @@ struct tree {
 	struct object object;
 	void *buffer;
 	unsigned long size;
-	struct tree_entry_list *entries;
 };
 
+struct tree_entry_list *create_tree_entry_list(struct tree *);
+void free_tree_entry_list(struct tree_entry_list *);
+
 struct tree *lookup_tree(const unsigned char *sha1);
 
 int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size);

← back to recent threads