René Scharfe <rene.scha...@lsrfire.ath.cx> writes:

>> 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)?

I do not think so.  These bits are needed per recursion level, and
it cannot be shoved into unpack_trees_options so I suspect that your
patch is the best we can do.  Or, perhaps we can

 - add df_conflict to struct unpack_trees_options;

 - have traverse_info->data point at struct unpack_trees_options as
   before; and

 - save the old value of o->df_conflict on the stack of
   traverse_trees_recursive(), update the field in place, and
   restore it when the recursion returns???

> ---
>  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to