Re: [PATCH v4 09/19] entry: add checkout_entry_ca() which takes preloaded conv_attrs

From: Christian Couder <christian.couder@gmail.com>
Date: Dec 6, 2020, 10:02 UTC
Message-ID: <CAP8UFD3SpzGbkeBas+yDEjDgtDiTdztD=6fMfG=8rHKN5B6kCQ@mail.gmail.com>
In-Reply-To: <aa635bda21c43e0d82ce21e791df280ee6231d43.1604521275.git.matheus.bernardino@usp.br>

On Wed, Nov 4, 2020 at 9:34 PM Matheus Tavares <matheus.bernardino@usp.br> wrote:

The title might be a bit shorter with: s/which takes/taking/ or s/which takes/using/

Show 9 quoted lines

> @@ -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.

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

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