Am 16.06.2013 02:56, schrieb Junio C Hamano:
> One thing renaming df_conficts to conflicts really proves is that
> this field is not used by the traverse_trees machinery at all.
>
> Before this patch, the bits in conflicts (now df_conflicts) mask had
> the semantics that is not consistent with the dirmask/mask the
> tree-walk machinery uses to communicate with its callers and
> callbacks. Everything in tree-walk.[ch] is about "walk N trees",
> and bit 0 of dirmask and mask always means the first tree, not
> "first tree, or in index if the callback is doing a merge", which
> is used in the conflicts field before this patch.
Right.
> I think the true source of the confusion is that the "conflicts"
> field does not logically belong there. It is not needed in the
> general "walk N trees" machinery.
NB: The only other caller of traverse_trees is git merge-tree.
> The information is only useful for the unpack_trees callback, and
> "info->data" is a more appropriate place to hang such a callback
> specific data.
>
> Perhaps we should use info->data field to point at
>
> struct {
> struct unpack_trees_options *o;
> unsigned long df_conflict;
> };
>
> and get rid of info->conflicts field?
Here's a patch that does so, but it complicates matters quite a bit.
Did I miss anything (or rather: add too much)?
---
tree-walk.h | 1 -
unpack-trees.c | 38 +++++++++++++++++++++++++++++---------
2 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/tree-walk.h b/tree-walk.h
index ae04b64..4876695 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -46,7 +46,6 @@ struct traverse_info {
int pathlen;
struct pathspec *pathspec;
- unsigned long df_conflicts;
traverse_callback_t fn;
void *data;
int show_all_errors;
diff --git a/unpack-trees.c b/unpack-trees.c
index b27f2a6..1c1b4de 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -416,11 +416,17 @@ static int unpack_index_entry(struct cache_entry *ce,
return ret;
}
+struct unpack_callback_info {
+ struct unpack_trees_options *options;
+ unsigned long df_conflicts;
+};
+
static int find_cache_pos(struct traverse_info *, const struct name_entry *);
static void restore_cache_bottom(struct traverse_info *info, int bottom)
{
- struct unpack_trees_options *o = info->data;
+ struct unpack_callback_info *uci = info->data;
+ struct unpack_trees_options *o = uci->options;
if (o->diff_index_cached)
return;
@@ -429,7 +435,8 @@ static void restore_cache_bottom(struct traverse_info *info, int bottom)
static int switch_cache_bottom(struct traverse_info *info)
{
- struct unpack_trees_options *o = info->data;
+ struct unpack_callback_info *uci = info->data;
+ struct unpack_trees_options *o = uci->options;
int ret, pos;
if (o->diff_index_cached)
@@ -452,9 +459,14 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
int i, ret, bottom;
struct tree_desc t[MAX_UNPACK_TREES];
void *buf[MAX_UNPACK_TREES];
+ struct unpack_callback_info *uci = info->data;
+ struct unpack_callback_info newuci;
struct traverse_info newinfo;
struct name_entry *p;
+ newuci = *uci;
+ newuci.df_conflicts |= df_conflicts;
+
p = names;
while (!p->mode)
p++;
@@ -464,7 +476,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
newinfo.pathspec = info->pathspec;
newinfo.name = *p;
newinfo.pathlen += tree_entry_len(p) + 1;
- newinfo.df_conflicts |= df_conflicts;
+ newinfo.data = &newuci;
for (i = 0; i < n; i++, dirmask >>= 1) {
const unsigned char *sha1 = NULL;
@@ -564,8 +576,9 @@ static int unpack_nondirectories(int n, unsigned long mask,
const struct traverse_info *info)
{
int i;
- struct unpack_trees_options *o = info->data;
- unsigned long conflicts = info->df_conflicts | dirmask;
+ struct unpack_callback_info *uci = info->data;
+ struct unpack_trees_options *o = uci->options;
+ unsigned long conflicts = uci->df_conflicts | dirmask;
/* Do we have *only* directories? Nothing to do */
if (mask == dirmask && !src[0])
@@ -644,7 +657,8 @@ static int find_cache_pos(struct traverse_info *info,
const struct name_entry *p)
{
int pos;
- struct unpack_trees_options *o = info->data;
+ struct unpack_callback_info *uci = info->data;
+ struct unpack_trees_options *o = uci->options;
struct index_state *index = o->src_index;
int pfxlen = info->pathlen;
int p_len = tree_entry_len(p);
@@ -699,7 +713,8 @@ static struct cache_entry *find_cache_entry(struct traverse_info *info,
const struct name_entry *p)
{
int pos = find_cache_pos(info, p);
- struct unpack_trees_options *o = info->data;
+ struct unpack_callback_info *uci = info->data;
+ struct unpack_trees_options *o = uci->options;
if (0 <= pos)
return o->src_index->cache[pos];
@@ -742,7 +757,8 @@ static void debug_unpack_callback(int n,
static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, struct name_entry *names, struct traverse_info *info)
{
struct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, };
- struct unpack_trees_options *o = info->data;
+ struct unpack_callback_info *uci = info->data;
+ struct unpack_trees_options *o = uci->options;
const struct name_entry *p = names;
/* Find first entry with a real name (we could use "mask" too) */
@@ -1054,11 +1070,15 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
if (len) {
const char *prefix = o->prefix ? o->prefix : "";
+ struct unpack_callback_info uci;
struct traverse_info info;
+ memset(&uci, 0, sizeof(uci));
+ uci.options = o;
+
setup_traverse_info(&info, prefix);
info.fn = unpack_callback;
- info.data = o;
+ info.data = &uci;
info.show_all_errors = o->show_all_errors;
info.pathspec = o->pathspec;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html