tree-walk: drop oid from traverse_info

As the previous commit shows, the presence of an oid in each level of the traverse_info is confusing and ultimately not necessary. Let's drop it to make it clear that it will not always be set (as well as convince us that it's unused, and let the compiler catch any merges with other branches that do add new uses). Since the oid is part of name_entry, we'll actually stop embedding a name_entry entirely, and instead just separately hold the pathname, its length, and the mode. This makes the resulting code slightly more verbose as we have to pass those elements around individually. But it also makes it more clear what each code path is going to use (and in most of the paths, we really only care about the pathname itself). A few of these conversions are noisier than they need to be, as they also take the opportunity to rename "len" to "namelen" for clarity (especially where we also have "pathlen" or "ce_len" alongside). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jul 31, 2019 at 00:38 UTC 9055384710dd8963b125f4f87c24d8f67d9fa24f
5 files changed +49 -39
builtin/merge-tree.c
+1 -1
@@ -181,7 +181,7 @@ static struct merge_list *create_entry(unsigned stage, unsigned mode, const stru
181 static char *traverse_path(const struct traverse_info *info, const struct name_entry *n)
182 {
183 char *path = xmallocz(traverse_path_len(info, n));
184 - return make_traverse_path(path, info, n);
184 + return make_traverse_path(path, info, n->path, n->pathlen);
185 }
186
187 static void resolve(const struct traverse_info *info, struct name_entry *ours, struct name_entry *result)
cache-tree.c
+1 -1
@@ -713,7 +713,7 @@ static struct cache_tree *find_cache_tree_from_traversal(struct cache_tree *root
713 if (!info->prev)
714 return root;
715 our_parent = find_cache_tree_from_traversal(root, info->prev);
716 - return cache_tree_find(our_parent, info->name.path);
716 + return cache_tree_find(our_parent, info->name);
717 }
718
719 int cache_tree_matches_traversal(struct cache_tree *root,
tree-walk.c
+12 -11
@@ -175,27 +175,27 @@ void setup_traverse_info(struct traverse_info *info, const char *base)
175 if (pathlen && base[pathlen-1] == '/')
176 pathlen--;
177 info->pathlen = pathlen ? pathlen + 1 : 0;
178 - info->name.path = base;
179 - info->name.pathlen = pathlen;
178 + info->name = base;
179 + info->namelen = pathlen;
180 if (pathlen)
181 info->prev = &dummy;
182 }
183
184 -char *make_traverse_path(char *path, const struct traverse_info *info, const struct name_entry *n)
184 +char *make_traverse_path(char *path, const struct traverse_info *info,
185 + const char *name, size_t namelen)
186 {
186 - int len = tree_entry_len(n);
187 int pathlen = info->pathlen;
188
189 - path[pathlen + len] = 0;
189 + path[pathlen + namelen] = 0;
190 for (;;) {
191 - memcpy(path + pathlen, n->path, len);
191 + memcpy(path + pathlen, name, namelen);
192 if (!pathlen)
193 break;
194 path[--pathlen] = '/';
195 - n = &info->name;
196 - len = tree_entry_len(n);
195 + name = info->name;
196 + namelen = info->namelen;
197 info = info->prev;
198 - pathlen -= len;
198 + pathlen -= namelen;
199 }
200 return path;
201 }
@@ -397,12 +397,13 @@ int traverse_trees(struct index_state *istate,
397
398 if (info->prev) {
399 strbuf_grow(&base, info->pathlen);
400 - make_traverse_path(base.buf, info->prev, &info->name);
400 + make_traverse_path(base.buf, info->prev, info->name,
401 + info->namelen);
402 base.buf[info->pathlen-1] = '/';
403 strbuf_setlen(&base, info->pathlen);
404 traverse_path = xstrndup(base.buf, info->pathlen);
405 } else {
405 - traverse_path = xstrndup(info->name.path, info->pathlen);
406 + traverse_path = xstrndup(info->name, info->pathlen);
407 }
408 info->traverse_path = traverse_path;
409 for (;;) {
tree-walk.h
+6 -2
@@ -56,7 +56,10 @@ enum get_oid_result get_tree_entry_follow_symlinks(struct object_id *tree_oid, c
56 struct traverse_info {
57 const char *traverse_path;
58 struct traverse_info *prev;
59 - struct name_entry name;
59 + const char *name;
60 + size_t namelen;
61 + unsigned mode;
62 +
63 int pathlen;
64 struct pathspec *pathspec;
65
@@ -67,7 +70,8 @@ struct traverse_info {
70 };
71
72 int get_tree_entry(const struct object_id *, const char *, struct object_id *, unsigned short *);
70 -char *make_traverse_path(char *path, const struct traverse_info *info, const struct name_entry *n);
73 +char *make_traverse_path(char *path, const struct traverse_info *info,
74 + const char *name, size_t namelen);
75 void setup_traverse_info(struct traverse_info *info, const char *base);
76
77 static inline int traverse_path_len(const struct traverse_info *info, const struct name_entry *n)
unpack-trees.c
+29 -24
@@ -632,7 +632,7 @@ static int unpack_index_entry(struct cache_entry *ce,
632 return ret;
633 }
634
635 -static int find_cache_pos(struct traverse_info *, const struct name_entry *);
635 +static int find_cache_pos(struct traverse_info *, const char *p, size_t len);
636
637 static void restore_cache_bottom(struct traverse_info *info, int bottom)
638 {
@@ -651,7 +651,7 @@ static int switch_cache_bottom(struct traverse_info *info)
651 if (o->diff_index_cached)
652 return 0;
653 ret = o->cache_bottom;
654 - pos = find_cache_pos(info->prev, &info->name);
654 + pos = find_cache_pos(info->prev, info->name, info->namelen);
655
656 if (pos < -1)
657 o->cache_bottom = -2 - pos;
@@ -690,7 +690,7 @@ static int index_pos_by_traverse_info(struct name_entry *names,
690 char *name = xmalloc(len + 1 /* slash */ + 1 /* NUL */);
691 int pos;
692
693 - make_traverse_path(name, info, names);
693 + make_traverse_path(name, info, names->path, names->pathlen);
694 name[len++] = '/';
695 name[len] = '\0';
696 pos = index_name_pos(o->src_index, name, len);
@@ -811,7 +811,9 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
811 newinfo = *info;
812 newinfo.prev = info;
813 newinfo.pathspec = info->pathspec;
814 - newinfo.name = *p;
814 + newinfo.name = p->path;
815 + newinfo.namelen = p->pathlen;
816 + newinfo.mode = p->mode;
817 newinfo.pathlen += tree_entry_len(p) + 1;
818 newinfo.df_conflicts |= df_conflicts;
819
@@ -863,14 +865,18 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
865 * itself - the caller needs to do the final check for the cache
866 * entry having more data at the end!
867 */
866 -static int do_compare_entry_piecewise(const struct cache_entry *ce, const struct traverse_info *info, const struct name_entry *n)
868 +static int do_compare_entry_piecewise(const struct cache_entry *ce,
869 + const struct traverse_info *info,
870 + const char *name, size_t namelen,
871 + unsigned mode)
872 {
868 - int len, pathlen, ce_len;
873 + int pathlen, ce_len;
874 const char *ce_name;
875
876 if (info->prev) {
877 int cmp = do_compare_entry_piecewise(ce, info->prev,
873 - &info->name);
878 + info->name, info->namelen,
879 + info->mode);
880 if (cmp)
881 return cmp;
882 }
@@ -884,15 +890,15 @@ static int do_compare_entry_piecewise(const struct cache_entry *ce, const struct
890 ce_len -= pathlen;
891 ce_name = ce->name + pathlen;
892
887 - len = tree_entry_len(n);
888 - return df_name_compare(ce_name, ce_len, S_IFREG, n->path, len, n->mode);
893 + return df_name_compare(ce_name, ce_len, S_IFREG, name, namelen, mode);
894 }
895
896 static int do_compare_entry(const struct cache_entry *ce,
897 const struct traverse_info *info,
893 - const struct name_entry *n)
898 + const char *name, size_t namelen,
899 + unsigned mode)
900 {
895 - int len, pathlen, ce_len;
901 + int pathlen, ce_len;
902 const char *ce_name;
903 int cmp;
904
@@ -902,7 +908,7 @@ static int do_compare_entry(const struct cache_entry *ce,
908 * it is quicker to use the precomputed version.
909 */
910 if (!info->traverse_path)
905 - return do_compare_entry_piecewise(ce, info, n);
911 + return do_compare_entry_piecewise(ce, info, name, namelen, mode);
912
913 cmp = strncmp(ce->name, info->traverse_path, info->pathlen);
914 if (cmp)
@@ -917,13 +923,12 @@ static int do_compare_entry(const struct cache_entry *ce,
923 ce_len -= pathlen;
924 ce_name = ce->name + pathlen;
925
920 - len = tree_entry_len(n);
921 - return df_name_compare(ce_name, ce_len, S_IFREG, n->path, len, n->mode);
926 + return df_name_compare(ce_name, ce_len, S_IFREG, name, namelen, mode);
927 }
928
929 static int compare_entry(const struct cache_entry *ce, const struct traverse_info *info, const struct name_entry *n)
930 {
926 - int cmp = do_compare_entry(ce, info, n);
931 + int cmp = do_compare_entry(ce, info, n->path, n->pathlen, n->mode);
932 if (cmp)
933 return cmp;
934
@@ -939,7 +944,8 @@ static int ce_in_traverse_path(const struct cache_entry *ce,
944 {
945 if (!info->prev)
946 return 1;
942 - if (do_compare_entry(ce, info->prev, &info->name))
947 + if (do_compare_entry(ce, info->prev,
948 + info->name, info->namelen, info->mode))
949 return 0;
950 /*
951 * If ce (blob) is the same name as the path (which is a tree
@@ -964,7 +970,7 @@ static struct cache_entry *create_ce_entry(const struct traverse_info *info,
970 ce->ce_flags = create_ce_flags(stage);
971 ce->ce_namelen = len;
972 oidcpy(&ce->oid, &n->oid);
967 - make_traverse_path(ce->name, info, n);
973 + make_traverse_path(ce->name, info, n->path, n->pathlen);
974
975 return ce;
976 }
@@ -1057,13 +1063,12 @@ static int unpack_failed(struct unpack_trees_options *o, const char *message)
1063 * the directory.
1064 */
1065 static int find_cache_pos(struct traverse_info *info,
1060 - const struct name_entry *p)
1066 + const char *p, size_t p_len)
1067 {
1068 int pos;
1069 struct unpack_trees_options *o = info->data;
1070 struct index_state *index = o->src_index;
1071 int pfxlen = info->pathlen;
1066 - int p_len = tree_entry_len(p);
1072
1073 for (pos = o->cache_bottom; pos < index->cache_nr; pos++) {
1074 const struct cache_entry *ce = index->cache[pos];
@@ -1099,7 +1104,7 @@ static int find_cache_pos(struct traverse_info *info,
1104 ce_len = ce_slash - ce_name;
1105 else
1106 ce_len = ce_namelen(ce) - pfxlen;
1102 - cmp = name_compare(p->path, p_len, ce_name, ce_len);
1107 + cmp = name_compare(p, p_len, ce_name, ce_len);
1108 /*
1109 * Exact match; if we have a directory we need to
1110 * delay returning it.
@@ -1114,7 +1119,7 @@ static int find_cache_pos(struct traverse_info *info,
1119 * E.g. ce_name == "t-i", and p->path == "t"; we may
1120 * have "t/a" in the index.
1121 */
1117 - if (p_len < ce_len && !memcmp(ce_name, p->path, p_len) &&
1122 + if (p_len < ce_len && !memcmp(ce_name, p, p_len) &&
1123 ce_name[p_len] < '/')
1124 continue; /* keep looking */
1125 break;
@@ -1125,7 +1130,7 @@ static int find_cache_pos(struct traverse_info *info,
1130 static struct cache_entry *find_cache_entry(struct traverse_info *info,
1131 const struct name_entry *p)
1132 {
1128 - int pos = find_cache_pos(info, p);
1133 + int pos = find_cache_pos(info, p->path, p->pathlen);
1134 struct unpack_trees_options *o = info->data;
1135
1136 if (0 <= pos)
@@ -1138,10 +1143,10 @@ static void debug_path(struct traverse_info *info)
1143 {
1144 if (info->prev) {
1145 debug_path(info->prev);
1141 - if (*info->prev->name.path)
1146 + if (*info->prev->name)
1147 putchar('/');
1148 }
1144 - printf("%s", info->name.path);
1149 + printf("%s", info->name);
1150 }
1151
1152 static void debug_name_entry(int i, struct name_entry *n)