Re: [PATCH] ls: issue error message on removed directory
On 12/02/2020 11:18, Pádraig Brady wrote:
On 11/02/2020 19:41, Pádraig Brady wrote:
On 11/02/2020 10:45, Colin Watson wrote:
If the current directory has been removed, then "ls" confusingly
produced no output and no error message, indistinguishable from the case
of running on an empty directory. It makes more sense to report ENOENT
in this case.
I don't think this is a general solution as the opendir() spec states:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/opendir.html
"The directory entries for dot and dot-dot are optional".
I've also seen reference to FUSE file systems that don't provide those entries.
In saying that, this is a common gotcha.
I wonder could we do something like:
+#ifdef __linux__
+ else if (! found_any_entries)
+{
+ /* If readdir finds no directory entries at all, not even "." or
+ "..", then double check that the directory exists. */
+ char buf[1024];
+ if (syscall(SYS_getdents, dirfd (dirp), buf, sizeof buf) == -1)
+{
+ file_failure (command_line_arg, _("reading directory %s"), name);
+ break;
+}
+}
+#endif
No objections so pushed at:
https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v8.31-88-g05a99f7d7
cheers,
Pádraig
Re: [PATCH] ls: issue error message on removed directory
On 12/02/2020 11:56, Bernhard Voelker wrote:
On 2/11/20 8:41 PM, Pádraig Brady wrote:
On 11/02/2020 10:45, Colin Watson wrote:
If the current directory has been removed, then "ls" confusingly
produced no output and no error message, indistinguishable from the case
of running on an empty directory. It makes more sense to report ENOENT
in this case.
I don't think this is a general solution as the opendir() spec states:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/opendir.html
"The directory entries for dot and dot-dot are optional".
I've also seen reference to FUSE file systems that don't provide those entries.
ls(1) does already run into ENOENT:
$ strace /usr/bin/ls
[...snipped...]
openat(AT_FDCWD, ".", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = 3
fstat(3, {st_mode=S_IFDIR|0755, st_size=0, ...}) = 0
getdents64(3, 0x55d8a0813f30, 32768)= -1 ENOENT (No such file or
directory)
close(3)= 0
close(1)= 0
close(2)= 0
exit_group(0) = ?
+++ exited with 0 +++
... so it seems that it only swallows errno, doesn't it?
Well it's readdir() in libc that's swallowing it (as per POSIX).
As mentioned previously, we should be able to query again like:
if (syscall(SYS_getdents, dirfd (dirp), NULL, 0) != EINVAL)
{
file_failure (command_line_arg, _("reading directory %s"), name);
break;
}
(note ENOENT is checked before EINVAL in the implementation,
and handling EINVAL like that simplifies the buffer considerations).
Also considering breaking shell scripts, I think this OK
as logic like the following would still work:
[ $(ls | wc -l) = 0 ] && exit # dir gone away
cheers,
Pádraig
Re: [PATCH] ls: issue error message on removed directory
On 12/02/2020 13:38, Pádraig Brady wrote:
On 12/02/2020 11:56, Bernhard Voelker wrote:
On 2/11/20 8:41 PM, Pádraig Brady wrote:
On 11/02/2020 10:45, Colin Watson wrote:
If the current directory has been removed, then "ls" confusingly
produced no output and no error message, indistinguishable from the case
of running on an empty directory. It makes more sense to report ENOENT
in this case.
I don't think this is a general solution as the opendir() spec states:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/opendir.html
"The directory entries for dot and dot-dot are optional".
I've also seen reference to FUSE file systems that don't provide those entries.
ls(1) does already run into ENOENT:
$ strace /usr/bin/ls
[...snipped...]
openat(AT_FDCWD, ".", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = 3
fstat(3, {st_mode=S_IFDIR|0755, st_size=0, ...}) = 0
getdents64(3, 0x55d8a0813f30, 32768)= -1 ENOENT (No such file or
directory)
close(3)= 0
close(1)= 0
close(2)= 0
exit_group(0) = ?
+++ exited with 0 +++
... so it seems that it only swallows errno, doesn't it?
Well it's readdir() in libc that's swallowing it (as per POSIX).
As mentioned previously, we should be able to query again like:
if (syscall(SYS_getdents, dirfd (dirp), NULL, 0) != EINVAL)
That should of course be:
if (syscall(SYS_getdents, dirfd (dirp), NULL, 0) == -1 && errno != EINVAL)
Re: [PATCH] ls: issue error message on removed directory
On 2/11/20 8:41 PM, Pádraig Brady wrote:
On 11/02/2020 10:45, Colin Watson wrote:
If the current directory has been removed, then "ls" confusingly
produced no output and no error message, indistinguishable from the case
of running on an empty directory. It makes more sense to report ENOENT
in this case.
I don't think this is a general solution as the opendir() spec states:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/opendir.html
"The directory entries for dot and dot-dot are optional".
I've also seen reference to FUSE file systems that don't provide those entries.
ls(1) does already run into ENOENT:
$ strace /usr/bin/ls
[...snipped...]
openat(AT_FDCWD, ".", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = 3
fstat(3, {st_mode=S_IFDIR|0755, st_size=0, ...}) = 0
getdents64(3, 0x55d8a0813f30, 32768)= -1 ENOENT (No such file or
directory)
close(3)= 0
close(1)= 0
close(2)= 0
exit_group(0) = ?
+++ exited with 0 +++
... so it seems that it only swallows errno, doesn't it?
Have a nice day,
Berny
Re: [PATCH] ls: issue error message on removed directory
On Wed, Feb 12, 2020 at 10:54:43AM +, Pádraig Brady wrote: > Sorry I sent the following to the list only. > Please cc coreutils@gnu.org on any responses... > > On 11/02/2020 10:45, Colin Watson wrote: > > If the current directory has been removed, then "ls" confusingly > > produced no output and no error message, indistinguishable from the case > > of running on an empty directory. It makes more sense to report ENOENT > > in this case. > > I don't think this is a general solution as the opendir() spec states: > https://pubs.opengroup.org/onlinepubs/9699919799/functions/opendir.html > "The directory entries for dot and dot-dot are optional". > I've also seen reference to FUSE file systems that don't provide those > entries. Ah. Fair point; that is unfortunate. I do think this is a confusing problem and worth trying to solve, though. The only other idea I have is, if there are no entries in the directory, to fstat the directory fd and see if st_nlink == 0; if so, issue a warning message, perhaps more like "directory may have been removed" than "no such file or directory". There are probably still FUSE-like situations where that goes wrong, but IIRC naïve FUSE implementations tend to set st_nlink to 1 rather than 0. Might something like that be acceptable? -- Colin Watson [cjwat...@debian.org]
Re: [PATCH] ls: issue error message on removed directory
On 11/02/2020 19:41, Pádraig Brady wrote:
On 11/02/2020 10:45, Colin Watson wrote:
If the current directory has been removed, then "ls" confusingly
produced no output and no error message, indistinguishable from the case
of running on an empty directory. It makes more sense to report ENOENT
in this case.
I don't think this is a general solution as the opendir() spec states:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/opendir.html
"The directory entries for dot and dot-dot are optional".
I've also seen reference to FUSE file systems that don't provide those entries.
In saying that, this is a common gotcha.
I wonder could we do something like:
+#ifdef __linux__
+ else if (! found_any_entries)
+{
+ /* If readdir finds no directory entries at all, not even "." or
+ "..", then double check that the directory exists. */
+ char buf[1024];
+ if (syscall(SYS_getdents, dirfd (dirp), buf, sizeof buf) == -1)
+{
+ file_failure (command_line_arg, _("reading directory %s"), name);
+ break;
+}
+}
+#endif
cheers,
Pádraig
Re: [PATCH] ls: issue error message on removed directory
On 11/02/2020 10:45, Colin Watson wrote: If the current directory has been removed, then "ls" confusingly produced no output and no error message, indistinguishable from the case of running on an empty directory. It makes more sense to report ENOENT in this case. I don't think this is a general solution as the opendir() spec states: https://pubs.opengroup.org/onlinepubs/9699919799/functions/opendir.html "The directory entries for dot and dot-dot are optional". I've also seen reference to FUSE file systems that don't provide those entries. cheers, Pádraig
Re: [PATCH] ls: issue error message on removed directory
On Tue, Feb 11, 2020 at 10:45:46AM +, Colin Watson wrote: > If the current directory has been removed, then "ls" confusingly > produced no output and no error message, indistinguishable from the case > of running on an empty directory. It makes more sense to report ENOENT > in this case. > > Reported by Owen Thomas. This was originally reported at https://lists.ubuntu.com/archives/ubuntu-users/2019-December/298805.html (thread continues at https://lists.ubuntu.com/archives/ubuntu-users/2020-February/299238.html). -- Colin Watson [cjwat...@debian.org]
