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