Re: [PATCH v2 06/20] merge-ort: implement a very basic collect_merge_info()
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Nov 9, 2020, 20:59 UTC
- Message-ID
- <20201109205955.2523889-1-jonathantanmy@google.com>
- In-Reply-To
- <CABPp-BFkAu6oO5V0jRh=ExuPxBXma1F17zaq-zEaqMagc72Ccw@mail.gmail.com>
Show 7 quoted lines
> > > + 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.
Show 20 quoted lines
> > 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.