GNU bug report logs - #63931
ls colors one symlink too much as non-broken in symlink chain

Previous Next

Package: coreutils;

Reported by: Martin Schulte <gnu <at> schrader-schulte.de>

Date: Tue, 6 Jun 2023 17:25:02 UTC

Severity: normal

Done: Pádraig Brady <P <at> draigBrady.com>

Bug is archived. No further changes may be made.

Full log


Message #22 received at 63931-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Martin Schulte <gnu <at> schrader-schulte.de>
Cc: 63931-done <at> debbugs.gnu.org
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
[Message part 1 (text/plain, inline)]
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 (text/x-patch, attachment)]
[0002-ls-display-command-line-symlinks-that-return-ELOOP.patch (text/x-patch, attachment)]

This bug report was last modified 1 year and 348 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.