tree_entry(): new tree-walking helper function

This adds a "tree_entry()" function that combines the common operation of
doing a "tree_entry_extract()" + "update_tree_entry()".

It also has a simplified calling convention, designed for simple loops
that traverse over a whole tree: the arguments are pointers to the tree
descriptor and a name_entry structure to fill in, and it returns a boolean
"true" if there was an entry left to be gotten in the tree.

This allows tree traversal with

	struct tree_desc desc;
	struct name_entry entry;

	desc.buf = tree->buffer;
	desc.size = tree->size;
	while (tree_entry(&desc, &entry) {
		... use "entry.{path, sha1, mode, pathlen}" ...
	}

which is not only shorter than writing it out in full, it's hopefully less
error prone too.

[ It's actually a tad faster too - we don't need to recalculate the entry
  pathlength in both extract and update, but need to do it only once.
  Also, some callers can avoid doing a "strlen()" on the result, since
  it's returned as part of the name_entry structure.

  However, by now we're talking just 1% speedup on "git-rev-list --objects
  --all", and we're definitely at the point where tree walking is no
  longer the issue any more. ]

NOTE! Not everybody wants to use this new helper function, since some of
the tree walkers very much on purpose do the descriptor update separately
from the entry extraction. So the "extract + update" sequence still
remains as the core sequence, this is just a simplified interface.

We should probably add a silly two-line inline helper function for
initializing the descriptor from the "struct tree" too, just to cut down
on the noise from that common "desc" initializer.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>

diff --git a/builtin-grep.c b/builtin-grep.c
index 53de8a883607157c88d02a30f0320aa9e5c0581a..acc4eea36397754d317e131f999b7b77dad31e36 100644 (file)
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -578,11 +578,9 @@ static int grep_tree(struct grep_opt *opt, const char **paths,
                     struct tree_desc *tree,
                     const char *tree_name, const char *base)
 {
-       unsigned mode;
        int len;
        int hit = 0;
-       const char *path;
-       const unsigned char *sha1;
+       struct name_entry entry;
        char *down;
        char *path_buf = xmalloc(PATH_MAX + strlen(tree_name) + 100);
 
@@ -597,36 +595,32 @@ static int grep_tree(struct grep_opt *opt, const char **paths,
        }
        len = strlen(path_buf);
 
-       while (tree->size) {
-               int pathlen;
-               sha1 = tree_entry_extract(tree, &path, &mode);
-               pathlen = strlen(path);
-               strcpy(path_buf + len, path);
+       while (tree_entry(tree, &entry)) {
+               strcpy(path_buf + len, entry.path);
 
-               if (S_ISDIR(mode))
+               if (S_ISDIR(entry.mode))
                        /* Match "abc/" against pathspec to
                         * decide if we want to descend into "abc"
                         * directory.
                         */
-                       strcpy(path_buf + len + pathlen, "/");
+                       strcpy(path_buf + len + entry.pathlen, "/");
 
                if (!pathspec_matches(paths, down))
                        ;
-               else if (S_ISREG(mode))
-                       hit |= grep_sha1(opt, sha1, path_buf);
-               else if (S_ISDIR(mode)) {
+               else if (S_ISREG(entry.mode))
+                       hit |= grep_sha1(opt, entry.sha1, path_buf);
+               else if (S_ISDIR(entry.mode)) {
                        char type[20];
                        struct tree_desc sub;
                        void *data;
-                       data = read_sha1_file(sha1, type, &sub.size);
+                       data = read_sha1_file(entry.sha1, type, &sub.size);
                        if (!data)
                                die("unable to read tree (%s)",
-                                   sha1_to_hex(sha1));
+                                   sha1_to_hex(entry.sha1));
                        sub.buf = data;
                        hit |= grep_tree(opt, paths, &sub, tree_name, down);
                        free(data);
                }
-               update_tree_entry(tree);
        }
        return hit;
 }

diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 00cdb5a6d9388c3a20e106c2047e7f6580b05ec6..10afd469683184219d62ec15b9a62430d44b73db 100644 (file)
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -53,28 +53,23 @@ typedef int (*merge_fn_t)(struct cache_entry **src);
 static struct tree_entry_list *create_tree_entry_list(struct tree *tree)
 {
        struct tree_desc desc;
+       struct name_entry one;
        struct tree_entry_list *ret = NULL;
        struct tree_entry_list **list_p = &ret;
 
        desc.buf = tree->buffer;
        desc.size = tree->size;
 
-       while (desc.size) {
-               unsigned mode;
-               const char *path;
-               const unsigned char *sha1;
+       while (tree_entry(&desc, &one)) {
                struct tree_entry_list *entry;
 
-               sha1 = tree_entry_extract(&desc, &path, &mode);
-               update_tree_entry(&desc);
-
                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;
-               entry->symlink = S_ISLNK(mode) != 0;
+               entry->name = one.path;
+               entry->sha1 = one.sha1;
+               entry->mode = one.mode;
+               entry->directory = S_ISDIR(one.mode) != 0;
+               entry->executable = (one.mode & S_IXUSR) != 0;
+               entry->symlink = S_ISLNK(one.mode) != 0;
                entry->next = NULL;
 
                *list_p = entry;
@@ -820,27 +815,22 @@ static int read_cache_unmerged(void)
 static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree)
 {
        struct tree_desc desc;
+       struct name_entry entry;
        int cnt;
 
        memcpy(it->sha1, tree->object.sha1, 20);
        desc.buf = tree->buffer;
        desc.size = tree->size;
        cnt = 0;
-       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))
+       while (tree_entry(&desc, &entry)) {
+               if (!S_ISDIR(entry.mode))
                        cnt++;
                else {
                        struct cache_tree_sub *sub;
-                       struct tree *subtree = lookup_tree(sha1);
+                       struct tree *subtree = lookup_tree(entry.sha1);
                        if (!subtree->object.parsed)
                                parse_tree(subtree);
-                       sub = cache_tree_sub(it, name);
+                       sub = cache_tree_sub(it, entry.path);
                        sub->cache_tree = cache_tree();
                        prime_cache_tree_rec(sub->cache_tree, subtree);
                        cnt += sub->cache_tree->entry_count;

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 6e2b898cca7213d4c49e38bb431ac85ddea0e062..17c04b962d82701ee6fd17ad15e368e1c283af25 100644 (file)
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -114,6 +114,7 @@ static struct object_list **process_tree(struct tree *tree,
 {
        struct object *obj = &tree->object;
        struct tree_desc desc;
+       struct name_entry entry;
        struct name_path me;
 
        if (!revs.tree_objects)
@@ -132,18 +133,11 @@ static struct object_list **process_tree(struct tree *tree,
        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);
+       while (tree_entry(&desc, &entry)) {
+               if (S_ISDIR(entry.mode))
+                       p = process_tree(lookup_tree(entry.sha1), p, &me, name);
                else
-                       p = process_blob(lookup_blob(sha1), p, &me, name);
+                       p = process_blob(lookup_blob(entry.sha1), p, &me, name);
        }
        free(tree->buffer);
        tree->buffer = NULL;

diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index 2d5e06fb692968829df00e7166eaaeb9d5902119..5f740cf7025bc2a9b46656332c6a54e78ee737e4 100644 (file)
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -271,30 +271,25 @@ static void write_global_extended_header(const unsigned char *sha1)
 static void traverse_tree(struct tree_desc *tree, struct strbuf *path)
 {
        int pathlen = path->len;
+       struct name_entry entry;
 
-       while (tree->size) {
-               const char *name;
-               const unsigned char *sha1;
-               unsigned mode;
+       while (tree_entry(tree, &entry)) {
                void *eltbuf;
                char elttype[20];
                unsigned long eltsize;
 
-               sha1 = tree_entry_extract(tree, &name, &mode);
-               update_tree_entry(tree);
-
-               eltbuf = read_sha1_file(sha1, elttype, &eltsize);
+               eltbuf = read_sha1_file(entry.sha1, elttype, &eltsize);
                if (!eltbuf)
-                       die("cannot read %s", sha1_to_hex(sha1));
+                       die("cannot read %s", sha1_to_hex(entry.sha1));
 
                path->len = pathlen;
-               strbuf_append_string(path, name);
-               if (S_ISDIR(mode))
+               strbuf_append_string(path, entry.path);
+               if (S_ISDIR(entry.mode))
                        strbuf_append_string(path, "/");
 
-               write_entry(sha1, path, mode, eltbuf, eltsize);
+               write_entry(entry.sha1, path, entry.mode, eltbuf, eltsize);
 
-               if (S_ISDIR(mode)) {
+               if (S_ISDIR(entry.mode)) {
                        struct tree_desc subtree;
                        subtree.buf = eltbuf;
                        subtree.size = eltsize;

diff --git a/fetch.c b/fetch.c
index 976a5a459c466b05b2caf27a58a979598daf05d3..ec2d8c3d9b3db7fff74af6c149db9d93c6882efd 100644 (file)
--- a/fetch.c
+++ b/fetch.c
@@ -38,25 +38,19 @@ static int process(struct object *obj);
 static int process_tree(struct tree *tree)
 {
        struct tree_desc desc;
+       struct name_entry entry;
 
        if (parse_tree(tree))
                return -1;
 
        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)) {
-                       struct tree *tree = lookup_tree(sha1);
+       while (tree_entry(&desc, &entry)) {
+               if (S_ISDIR(entry.mode)) {
+                       struct tree *tree = lookup_tree(entry.sha1);
                        process_tree(tree);
                } else {
-                       struct blob *blob = lookup_blob(sha1);
+                       struct blob *blob = lookup_blob(entry.sha1);
                        process(&blob->object);
                }
        }

diff --git a/http-push.c b/http-push.c
index 72ad89ce11217b56f4fe4a722249dfaf9ca86ac3..b1c018a08993dfd7278f3db35b9a140ab0e166d6 100644 (file)
--- a/http-push.c
+++ b/http-push.c
@@ -1715,6 +1715,7 @@ static struct object_list **process_tree(struct tree *tree,
 {
        struct object *obj = &tree->object;
        struct tree_desc desc;
+       struct name_entry entry;
        struct name_path me;
 
        obj->flags |= LOCAL;
@@ -1734,18 +1735,11 @@ static struct object_list **process_tree(struct tree *tree,
        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);
+       while (tree_entry(&desc, &entry)) {
+               if (S_ISDIR(entry.mode))
+                       p = process_tree(lookup_tree(entry.sha1), p, &me, name);
                else
-                       p = process_blob(lookup_blob(sha1), p, &me, name);
+                       p = process_blob(lookup_blob(entry.sha1), p, &me, name);
        }
        free(tree->buffer);
        tree->buffer = NULL;

diff --git a/pack-objects.c b/pack-objects.c
index 77284cfdbaef108676c95bd4d9771e40080fd406..3590cd5e5983cde07016f7a1f20ce146b6a73703 100644 (file)
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -690,25 +690,20 @@ static void add_pbase_object(struct tree_desc *tree,
                             const char *name,
                             int cmplen)
 {
-       while (tree->size) {
-               const unsigned char *sha1;
-               const char *entry_name;
-               int entry_len;
-               unsigned mode;
+       struct name_entry entry;
+
+       while (tree_entry(tree,&entry)) {
                unsigned long size;
                char type[20];
 
-               sha1 = tree_entry_extract(tree, &entry_name, &mode);
-               update_tree_entry(tree);
-               entry_len = strlen(entry_name);
-               if (entry_len != cmplen ||
-                   memcmp(entry_name, name, cmplen) ||
-                   !has_sha1_file(sha1) ||
-                   sha1_object_info(sha1, type, &size))
+               if (entry.pathlen != cmplen ||
+                   memcmp(entry.path, name, cmplen) ||
+                   !has_sha1_file(entry.sha1) ||
+                   sha1_object_info(entry.sha1, type, &size))
                        continue;
                if (name[cmplen] != '/') {
                        unsigned hash = name_hash(up, name);
-                       add_object_entry(sha1, hash, 1);
+                       add_object_entry(entry.sha1, hash, 1);
                        return;
                }
                if (!strcmp(type, tree_type)) {
@@ -718,15 +713,15 @@ static void add_pbase_object(struct tree_desc *tree,
                        const char *down = name+cmplen+1;
                        int downlen = name_cmp_len(down);
 
-                       tree = pbase_tree_get(sha1);
+                       tree = pbase_tree_get(entry.sha1);
                        if (!tree)
                                return;
                        sub.buf = tree->tree_data;
                        sub.size = tree->tree_size;
 
                        me.up = up;
-                       me.elem = entry_name;
-                       me.len = entry_len;
+                       me.elem = entry.path;
+                       me.len = entry.pathlen;
                        add_pbase_object(&sub, &me, down, downlen);
                        pbase_tree_put(tree);
                }

diff --git a/revision.c b/revision.c
index 8e93e40bb379d7ba0f1c7d483581b5d16f4f1753..6a6952cd559af89d3c8dc2c477ddd8c25b470d66 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -54,6 +54,7 @@ static void mark_blob_uninteresting(struct blob *blob)
 void mark_tree_uninteresting(struct tree *tree)
 {
        struct tree_desc desc;
+       struct name_entry entry;
        struct object *obj = &tree->object;
 
        if (obj->flags & UNINTERESTING)
@@ -66,18 +67,11 @@ void mark_tree_uninteresting(struct tree *tree)
 
        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))
-                       mark_tree_uninteresting(lookup_tree(sha1));
+       while (tree_entry(&desc, &entry)) {
+               if (S_ISDIR(entry.mode))
+                       mark_tree_uninteresting(lookup_tree(entry.sha1));
                else
-                       mark_blob_uninteresting(lookup_blob(sha1));
+                       mark_blob_uninteresting(lookup_blob(entry.sha1));
        }
 
        /*

diff --git a/tree-walk.c b/tree-walk.c
index 39220582713311c692dde45d6705700995545f38..297c6972b9256578f9cd7a92404dda55c8e34a2e 100644 (file)
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -37,7 +37,7 @@ static void entry_extract(struct tree_desc *t, struct name_entry *a)
 
 void update_tree_entry(struct tree_desc *desc)
 {
-       void *buf = desc->buf;
+       const void *buf = desc->buf;
        unsigned long size = desc->size;
        int len = strlen(buf) + 1 + 20;
 
@@ -63,7 +63,7 @@ static const char *get_mode(const char *str, unsigned int *modep)
 
 const unsigned char *tree_entry_extract(struct tree_desc *desc, const char **pathp, unsigned int *modep)
 {
-       void *tree = desc->buf;
+       const void *tree = desc->buf;
        unsigned long size = desc->size;
        int len = strlen(tree)+1;
        const unsigned char *sha1 = tree + len;
@@ -78,6 +78,35 @@ const unsigned char *tree_entry_extract(struct tree_desc *desc, const char **pat
        return sha1;
 }
 
+int tree_entry(struct tree_desc *desc, struct name_entry *entry)
+{
+       const void *tree = desc->buf, *path;
+       unsigned long len, size = desc->size;
+
+       if (!size)
+               return 0;
+
+       path = get_mode(tree, &entry->mode);
+       if (!path)
+               die("corrupt tree file");
+
+       entry->path = path;
+       len = strlen(path);
+       entry->pathlen = len;
+
+       path += len + 1;
+       entry->sha1 = path;
+
+       path += 20;
+       len = path - tree;
+       if (len > size)
+               die("corrupt tree file");
+
+       desc->buf = path;
+       desc->size = size - len;
+       return 1;
+}
+
 void traverse_trees(int n, struct tree_desc *t, const char *base, traverse_callback_t callback)
 {
        struct name_entry *entry = xmalloc(n*sizeof(*entry));

diff --git a/tree-walk.h b/tree-walk.h
index 47438fe1c08a8e119fd08aea0eac612c76c21a66..e57befa4dac0854906d5b792ce546dab0e6dcdc3 100644 (file)
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -2,7 +2,7 @@
 #define TREE_WALK_H
 
 struct tree_desc {
-       void *buf;
+       const void *buf;
        unsigned long size;
 };
 
@@ -16,6 +16,9 @@ struct name_entry {
 void update_tree_entry(struct tree_desc *);
 const unsigned char *tree_entry_extract(struct tree_desc *, const char **, unsigned int *);
 
+/* Helper function that does both of the above and returns true for success */
+int tree_entry(struct tree_desc *, struct name_entry *);
+
 void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1);
 
 typedef void (*traverse_callback_t)(int n, unsigned long mask, struct name_entry *entry, const char *base);

diff --git a/tree.c b/tree.c
index fb187242596062efbf487f035b7508451ca7a426..9bbe2da37b24007b6409d5e1e5c61a20d0903628 100644 (file)
--- a/tree.c
+++ b/tree.c
@@ -79,6 +79,7 @@ int read_tree_recursive(struct tree *tree,
                        read_tree_fn_t fn)
 {
        struct tree_desc desc;
+       struct name_entry entry;
 
        if (parse_tree(tree))
                return -1;
@@ -86,18 +87,11 @@ int read_tree_recursive(struct tree *tree,
        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))
+       while (tree_entry(&desc, &entry)) {
+               if (!match_tree_entry(base, baselen, entry.path, entry.mode, match))
                        continue;
 
-               switch (fn(sha1, base, baselen, name, mode, stage)) {
+               switch (fn(entry.sha1, base, baselen, entry.path, entry.mode, stage)) {
                case 0:
                        continue;
                case READ_TREE_RECURSIVE:
@@ -105,18 +99,17 @@ int read_tree_recursive(struct tree *tree,
                default:
                        return -1;
                }
-               if (S_ISDIR(mode)) {
+               if (S_ISDIR(entry.mode)) {
                        int retval;
-                       int pathlen = strlen(name);
                        char *newbase;
 
-                       newbase = xmalloc(baselen + 1 + pathlen);
+                       newbase = xmalloc(baselen + 1 + entry.pathlen);
                        memcpy(newbase, base, baselen);
-                       memcpy(newbase + baselen, name, pathlen);
-                       newbase[baselen + pathlen] = '/';
-                       retval = read_tree_recursive(lookup_tree(sha1),
+                       memcpy(newbase + baselen, entry.path, entry.pathlen);
+                       newbase[baselen + entry.pathlen] = '/';
+                       retval = read_tree_recursive(lookup_tree(entry.sha1),
                                                     newbase,
-                                                    baselen + pathlen + 1,
+                                                    baselen + entry.pathlen + 1,
                                                     stage, match, fn);
                        free(newbase);
                        if (retval)
@@ -156,6 +149,7 @@ static int track_tree_refs(struct tree *item)
        int n_refs = 0, i;
        struct object_refs *refs;
        struct tree_desc desc;
+       struct name_entry entry;
 
        /* Count how many entries there are.. */
        desc.buf = item->buffer;
@@ -170,18 +164,13 @@ static int track_tree_refs(struct tree *item)
        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;
+       while (tree_entry(&desc, &entry)) {
                struct object *obj;
 
-               sha1 = tree_entry_extract(&desc, &name, &mode);
-               update_tree_entry(&desc);
-               if (S_ISDIR(mode))
-                       obj = &lookup_tree(sha1)->object;
+               if (S_ISDIR(entry.mode))
+                       obj = &lookup_tree(entry.sha1)->object;
                else
-                       obj = &lookup_blob(sha1)->object;
+                       obj = &lookup_blob(entry.sha1)->object;
                refs->ref[i++] = obj;
        }
        set_object_refs(&item->object, refs);