Re: [PATCH] unpack-trees: don't shift conflicts left and right

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



René Scharfe <rene.scharfe@xxxxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]