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

From: Pádraig Brady <[email-masked]>
Date: Wed, 7 Jun 2023 13:23:55 +0100

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;