From: Christian Couder <christian.couder@gmail.com>
Date: Sun, 06 Dec 2020 09:35:48 GMT
Subject: Re: [PATCH v4 08/19] entry: move conv_attrs lookup up to checkout_entry()
Message-ID: <CAP8UFD1-3YxT42K77Rd-XSRsoXYr_RM8Dj_r8-2GQ0noQw58iQ@mail.gmail.com>
In-Reply-To: <5c993cc27f67109828390c7856d6c03d4a2cbb32.1604521275.git.matheus.bernardino@usp.br>

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

> +       if (topath) {
> +               if (S_ISREG(ce->ce_mode)) {
> +                       convert_attrs(state->istate, &ca_buf, ce->name);
> +                       ca = &ca_buf;
> +               }
> +               return write_entry(ce, topath, ca, state, 1);

We pass `ca` here instead of `&ca_buf` because ca is NULL if we are
not dealing with a regular file. Ok, I think it's indeed better to
pass NULL in this case.

> @@ -517,9 +526,16 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state,
>                 return 0;
>
>         create_directories(path.buf, path.len, state);
> +
>         if (nr_checkouts)
>                 (*nr_checkouts)++;
> -       return write_entry(ce, path.buf, state, 0);
> +
> +       if (S_ISREG(ce->ce_mode)) {
> +               convert_attrs(state->istate, &ca_buf, ce->name);
> +               ca = &ca_buf;
> +       }
> +
> +       return write_entry(ce, path.buf, NULL, state, 0);

I am not sure why NULL is passed here though instead of `ca`.

The following comment is added in front of write_entry():

+/* Note: ca is used (and required) iff the entry refers to a regular file. */

So I would think that `ca` should be passed.