bug#63931: ls colors one symlink too much as non-broken in symlink chain

[Pádraig Brady]

On 07/06/2023 13:23, Pádraig Brady wrote:
> On 07/06/2023 10:56, Martin Schulte wrote:
> > Hello Paul, hello Pádraig,
> >
> > thanks a lot for your analysis and explanations!
> >
> > > With that in mind, the code change you proposed is reasonably innocuous,
> > > although it slows things down a bit in the usual case. Not sure it's
> > > worth doing (I guess it does fix a race but there are other unfixable
> > > races in this area....).
> >
> > I first supposed that the effect was caused by an off-by-one problem, but after 
> > understanding it, I think you are right here to ask if changing the behaviour 
> > is worth doing.
>
> Well changing it makes things more consistent,
> and also simplifies ls.c as we can remove the make_link_name() function.
>
> > Looking at
> >
> > $ ls {38..42}
> > ls: cannot access '41': Too many levels of symbolic links
> > ls: cannot access '42': Too many levels of symbolic links
> > 38  39  40
> >
> > shows that there would be more issues to consider in this case.
>
> Indeed. That can be addressed with the following.
>
> cheers,
> Pádraig
>
> diff --git a/src/ls.c b/src/ls.c
> index fbeb9b6dc..33f692bb4 100644
> --- a/src/ls.c
> +++ b/src/ls.c
> @@ -3480,7 +3480,7 @@ gobble_file (char const *name, enum filetype type, ino_t 
> inode,
>                    break;
>
>                  need_lstat = (err < 0
> -                            ? errno == ENOENT
> +                            ? (errno == ENOENT || errno == ELOOP)
>                                : ! S_ISDIR (f->stat.st_mode));
>                  if (!need_lstat)
>                    break;

The complete two patches in this area are attached,
which I'll apply a bit later.

Marking this as done,

thanks,
Pádraig

0001-ls-use-more-standard-symlink-traversal.patch
Description: Text Data

0002-ls-display-command-line-symlinks-that-return-ELOOP.patch
Description: Text Data