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

[Jonathan Tan <jonathantanmy@google.com>]

> > > +     unsigned mbase_null = !(mask & 1);
> > > +     unsigned side1_null = !(mask & 2);
> > > +     unsigned side2_null = !(mask & 4);
> >
> > Should these be "int"?
> 
> Does the type matter, particularly since "boolean" isn't available?

It doesn't, which is why I would expect the most generic type - if I see
something else, I would be led to think that there was a specific reason
for choosing that. But if I'm in the minority, that's fine.

> > 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 ""?
> 
> You're really digging in; I love it.  From setup_path_info(), the
> directory_name is set from the current_dir_name:
>         path_info->merged.directory_name = current_dir_name;
> (and if you follow where the current_dir_name parameter gets its value
> from, you find that it came indirectly from
> opt->priv->current_dir_name), so current_dir_name must meet all the
> requirements on merge_info's directory_name field.
> 
> Perhaps there's still some kind of additional simplification possible
> here, but directory rename detection is an area that has to take some
> special care around this requirement.  I simplified the code a little
> bit in this area as I was trying to break off a good first 20 patches
> to submit, but even if we can simplify it more, the structure is just
> going to come back later.

Ah, I see.