On Sun, Dec 6, 2020 at 7:03 AM Christian Couder
<christian.couder@xxxxxxxxx> wrote:
>
> On Wed, Nov 4, 2020 at 9:34 PM Matheus Tavares
> <matheus.bernardino@xxxxxx> wrote:
>
> The title might be a bit shorter with: s/which takes/taking/ or
> s/which takes/using/
>
> > @@ -530,12 +531,12 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state,
> > if (nr_checkouts)
> > (*nr_checkouts)++;
> >
> > - if (S_ISREG(ce->ce_mode)) {
> > + if (S_ISREG(ce->ce_mode) && !ca) {
> > convert_attrs(state->istate, &ca_buf, ce->name);
> > ca = &ca_buf;
> > }
>
> I wonder if it's possible to avoid repeating the above 4 lines twice
> in this function.
Yeah, I was thinking about that as well. But the only way I found to
avoid the repetition would be to place this block before check_path().
The problem is that we would end up calling convert_attrs() even for
the cases where we later decide not to checkout the entry (because,
for example, state->not_new is true or the file is already up to
date).
> > - return write_entry(ce, path.buf, NULL, state, 0);
> > + return write_entry(ce, path.buf, ca, state, 0);
>
> It's good that ca is eventually passed here, but it might belong to
> the previous patch.
Right, I'll fix the previous patch.
> > -int checkout_entry(struct cache_entry *ce, const struct checkout *state,
> > - char *topath, int *nr_checkouts);
> > +#define checkout_entry(ce, state, topath, nr_checkouts) \
> > + checkout_entry_ca(ce, NULL, state, topath, nr_checkouts)
>
> I thought that we prefer inline functions over macros when possible.
Thanks, I didn't know about the preference. I'll change that.
