Re: [PATCH v2 06/20] merge-ort: implement a very basic collect_merge_info()

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

 



> diff --git a/merge-ort.c b/merge-ort.c
> index 537da9f6df..626eb9713e 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -77,13 +77,130 @@ static int err(struct merge_options *opt, const char *err, ...)
>  	return -1;
>  }
>  
> +static int collect_merge_info_callback(int n,
> +				       unsigned long mask,
> +				       unsigned long dirmask,
> +				       struct name_entry *names,
> +				       struct traverse_info *info)
> +{

[snip]

> +	unsigned mbase_null = !(mask & 1);
> +	unsigned side1_null = !(mask & 2);
> +	unsigned side2_null = !(mask & 4);

Should these be "int"?

> +	/*
> +	 * A bunch of sanity checks verifying that traverse_trees() calls
> +	 * us the way I expect.  Could just remove these at some point,
> +	 * though maybe they are helpful to future code readers.
> +	 */
> +	assert(mbase_null == is_null_oid(&names[0].oid));
> +	assert(side1_null == is_null_oid(&names[1].oid));
> +	assert(side2_null == is_null_oid(&names[2].oid));
> +	assert(!mbase_null || !side1_null || !side2_null);
> +	assert(mask > 0 && mask < 8);

These were helpful to me.

> +	/* Other invariant checks, mostly for documentation purposes. */
> +	assert(mask == (dirmask | filemask));

But not this - filemask was computed in this function, so I need not
look elsewhere to see that this is correct.

> +	/*
> +	 * TODO: record information about the path other than all zeros,
> +	 * so we can resolve later in process_entries.
> +	 */
> +	ci = xcalloc(1, sizeof(struct conflict_info));
> +	strmap_put(&opti->paths, fullpath, ci);

OK - so each entry is a full-size conflict_info to store all relevant
information. Presumably some of these will be converted later into what
is effectively a struct merged_info (so, the extra struct conflict_info
fields are unused but memory is still occupied).

I do see that in patch 10, there is an optimization that directly
allocates the smaller struct merged_info when it is known at this point
that there is no conflict.

[snip rest of function]

>  static int collect_merge_info(struct merge_options *opt,
>  			      struct tree *merge_base,
>  			      struct tree *side1,
>  			      struct tree *side2)
>  {
> -	/* TODO: Implement this using traverse_trees() */
> -	die("Not yet implemented.");
> +	int ret;
> +	struct tree_desc t[3];
> +	struct traverse_info info;
> +	char *toplevel_dir_placeholder = "";
> +
> +	opt->priv->current_dir_name = toplevel_dir_placeholder;
> +	setup_traverse_info(&info, toplevel_dir_placeholder);

I thought that this was written like this (instead of inlining the 2
double-quotes) to ensure that the string-equality-is-pointer-equality
characteristic holds, but I see that that characteristic is for
directory_name in struct merged_info, not current_dir_name in struct
merge_options_internal. Any reason for not inlining ""?

[snip rest of function]



[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]

  Powered by Linux