From: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Date: Mon, 07 Dec 2020 13:52:29 GMT
Subject: Re: [PATCH v4 08/19] entry: move conv_attrs lookup up to checkout_entry()
Message-ID: <CAHd-oW5Mj13_4rgHrQ-Awtzg0i0Wkbeb78z466FO=_d72New=A@mail.gmail.com>
In-Reply-To: <CAP8UFD1-3YxT42K77Rd-XSRsoXYr_RM8Dj_r8-2GQ0noQw58iQ@mail.gmail.com>

On Sun, Dec 6, 2020 at 6:36 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> 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`.

Oops, this is indeed wrong. I think I forgot to modify this line while
applying the changes from the last review round. Thanks for catching
it!