Re: [GSoC][PATCH 3/3] clone: use dir-iterator to avoid explicit dir traversal
- From
Matheus Tavares Bernardino <matheus.bernardino@usp.br>
- Date
- Feb 24, 2019, 18:19 UTC
- Message-ID
- <CAHd-oW71y7nadq96c_4WDgs=xuTy-Kd98arJxv7eMwvZ3LkPXg@mail.gmail.com>
- In-Reply-To
- <20190223214856.GQ6085@hank.intra.tgummerer.com>
On Sat, Feb 23, 2019 at 6:48 PM Thomas Gummerer <t.gummerer@gmail.com> wrote:
Show 8 quoted lines
> > On 02/23, Matheus Tavares wrote: > > --- > > Changes in v2: > > - Improved patch message > > - Removed a now unused variable > > s/variable/parameter/ I believe?
Yes, you are right!
Show 26 quoted lines
> > + while ((iter_status = dir_iterator_advance(iter)) == ITER_OK) {
> > strbuf_setlen(src, src_len);
> > - strbuf_addstr(src, de->d_name);
> > + strbuf_addstr(src, iter->relative_path);
> > strbuf_setlen(dest, dest_len);
> > - strbuf_addstr(dest, de->d_name);
> > - if (stat(src->buf, &buf)) {
> > + strbuf_addstr(dest, iter->relative_path);
> > +
> > + /*
> > + * dir_iterator_advance already calls lstat to populate iter->st
> > + * but, unlike stat, lstat does not checks for permissions on
> > + * the given path.
> > + */
>
> Hmm, lstat does check the permissions on the path, it just doesn't
> follow symlinks. I think I actually mislead you in my previous review
> here, and was reading the code in dir-iterator.c all wrong.
>
> I thought it said "if (errno == ENOENT) warning(...)", however the
> condition is "errno != ENOENT", which is why I thought we were loosing
> warnings when errno == EACCES for example.
>
> As we decided that we would no longer follow symlinks now, I think we
> can actually get rid of the stat call here. Sorry about the confusion.
>Ok, I also read the lstat man page wrongly and though it didn't check for permissions. Thanks for noticing that. I will remove the lstat call in v3.