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

[GNU bug Tracking System]

Your message dated Wed, 7 Jun 2023 16:01:42 +0100
with message-id <7a38b7fc-bf5e-2f42-2a77-9eb3d71dec79@draigBrady.com>
and subject line Re: bug#63931: ls colors one symlink too much as non-broken in 
symlink chain
has caused the debbugs.gnu.org bug report #63931,
regarding ls colors one symlink too much as non-broken in symlink chain
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs@gnu.org.)


-- 
63931: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63931
GNU Bug Tracking System
Contact help-debbugs@gnu.org with problems
> --- Begin Message ---
>
>
>
> Subject: 
>
> ls colors one symlink too much as non-broken in symlink chain
>
>
>
>
> Date: 
>
> Tue, 6 Jun 2023 19:24:26 +0200
>
>
>
>
> Hello coreutils-maintainers,
>
> I create a long chain of symlinks - ls colors the 41st element as ok while the 
> kernel already gives up after 40 symlinks:
>
> $ uname -a
> Linux martnix4 5.10.0-23-amd64 #1 SMP Debian 5.10.179-1 (2023-05-12) x86_64 
> GNU/Linux
> $ ls --version | head -n 1
> ls (GNU coreutils) 8.32
> $ mkdir empty
> $ cd empty
> $ last=00 ; touch $last ; for ln in {01..50}; do ln -s $last $ln; last=$ln; done
> $ ls --color -l {38..42}
> lrwxrwxrwx 1 u g 2 Jun  6 19:00 38 -> 37   <- 38 colored as symlink
> lrwxrwxrwx 1 u g 2 Jun  6 19:00 39 -> 38   <- 39 colored as symlink
> lrwxrwxrwx 1 u g 2 Jun  6 19:00 40 -> 39   <- 40 colored as symlink
> lrwxrwxrwx 1 u g 2 Jun  6 19:00 41 -> 40   <- 41 colored as symlink
> lrwxrwxrwx 1 u g 2 Jun  6 19:00 42 -> 41   <- 42 and 41 colored as broken link
> $ cat 40
> $ cat 41
> cat: 41: Too many levels of symbolic links
>
> This could be reproduced with "ls (GNU coreutils) 9.3.45-6a618" compiled from 
> the git repository.
>
> Best regards,
>
> Martin
>
>
>
> --- End Message ---

--- Begin Message --- Subject: Re: bug#63931: ls colors one symlink too much as non-broken in symlink chain Date: Wed, 7 Jun 2023 16:01:42 +0100 User-agent: Mozilla Thunderbird

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

--- End Message ---