On Sat, Feb 23, 2019 at 6:48 PM Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
>
> 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!
> > + 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.
