Improve error for redirections to invalid paths

This finds the first broken component, to help people figure out where
they misspelt something.

E.g.

```
echo foo >/usr/lob/systemd/system/machines.target.wants/var-lib-machines.mount
```

will now show:

```
warning: Path '/usr/lob' does not exist
```

which would help with seeing that it should be "/usr/lib".
This commit is contained in:
Fabian Homborg 2021-11-19 21:11:28 +01:00
parent c82ce5132b
commit 8391f94081
5 changed files with 38 additions and 9 deletions

View file

@ -7,6 +7,7 @@
#include <fcntl.h> #include <fcntl.h>
#include <stddef.h> #include <stddef.h>
#include <stdio.h> #include <stdio.h>
#include <sys/stat.h>
#include <unistd.h> #include <unistd.h>
#include <cstring> #include <cstring>
@ -236,8 +237,32 @@ bool io_chain_t::append_from_specs(const redirection_spec_list_t &specs, const w
if ((oflags & O_EXCL) && (errno == EEXIST)) { if ((oflags & O_EXCL) && (errno == EEXIST)) {
FLOGF(warning, NOCLOB_ERROR, spec.target.c_str()); FLOGF(warning, NOCLOB_ERROR, spec.target.c_str());
} else { } else {
if (should_flog(warning)) {
FLOGF(warning, FILE_ERROR, spec.target.c_str()); FLOGF(warning, FILE_ERROR, spec.target.c_str());
if (should_flog(warning)) wperror(L"open"); auto err = errno;
// If the error is that the file doesn't exist
// or there's a non-directory component,
// find the first problematic component for a better message.
if (err == ENOENT || err == ENOTDIR) {
auto dname = spec.target;
struct stat buf;
while (!dname.empty()) {
auto next = wdirname(dname);
if (!wstat(next, &buf)) {
if (!S_ISDIR(buf.st_mode)) {
FLOGF(warning, _(L"Path '%ls' is not a directory"), next.c_str());
} else {
FLOGF(warning, _(L"Path '%ls' does not exist"), dname.c_str());
}
break;
}
dname = next;
}
} else {
wperror(L"open");
}
}
} }
// If opening a file fails, insert a closed FD instead of the file redirection // If opening a file fails, insert a closed FD instead of the file redirection
// and return false. This lets execution potentially recover and at least gives // and return false. This lets execution potentially recover and at least gives

View file

@ -1,12 +1,12 @@
#RUN: %fish %s #RUN: %fish %s
exec cat <nosuchfile exec cat <nosuchfile
#CHECKERR: warning: An error occurred while redirecting file 'nosuchfile' #CHECKERR: warning: An error occurred while redirecting file 'nosuchfile'
#CHECKERR: open: No such file or directory #CHECKERR: warning: Path 'nosuchfile' does not exist
echo "failed: $status" echo "failed: $status"
#CHECK: failed: 1 #CHECK: failed: 1
not exec cat <nosuchfile not exec cat <nosuchfile
#CHECKERR: warning: An error occurred while redirecting file 'nosuchfile' #CHECKERR: warning: An error occurred while redirecting file 'nosuchfile'
#CHECKERR: open: No such file or directory #CHECKERR: warning: Path 'nosuchfile' does not exist
echo "neg failed: $status" echo "neg failed: $status"
#CHECK: neg failed: 0 #CHECK: neg failed: 0

View file

@ -181,24 +181,24 @@ command true > /not/a/valid/path
echo $pipestatus : $status echo $pipestatus : $status
#CHECK: 1 : 1 #CHECK: 1 : 1
#CHECKERR: warning: An error occurred while redirecting file '/not/a/valid/path' #CHECKERR: warning: An error occurred while redirecting file '/not/a/valid/path'
#CHECKERR: open: No such file or directory #CHECKERR: warning: Path '/not' does not exist
# Here the first process will launch, the second one will not. # Here the first process will launch, the second one will not.
command true | command true | command true > /not/a/valid/path command true | command true | command true > /not/a/valid/path
echo $pipestatus : $status echo $pipestatus : $status
#CHECK: 0 0 1 : 1 #CHECK: 0 0 1 : 1
#CHECKERR: warning: An error occurred while redirecting file '/not/a/valid/path' #CHECKERR: warning: An error occurred while redirecting file '/not/a/valid/path'
#CHECKERR: open: No such file or directory #CHECKERR: warning: Path '/not' does not exist
# Pipeline breaks do not result in dangling jobs. # Pipeline breaks do not result in dangling jobs.
command true | command cat > /not/a/valid/path ; jobs command true | command cat > /not/a/valid/path ; jobs
#CHECKERR: warning: An error occurred while redirecting file '/not/a/valid/path' #CHECKERR: warning: An error occurred while redirecting file '/not/a/valid/path'
#CHECKERR: open: No such file or directory #CHECKERR: warning: Path '/not' does not exist
#CHECK: jobs: There are no jobs #CHECK: jobs: There are no jobs
# Regression test for #7038 # Regression test for #7038
cat /dev/zero | dd > /not/a/valid/path cat /dev/zero | dd > /not/a/valid/path
echo 'Not hung' echo 'Not hung'
#CHECKERR: warning: An error occurred while redirecting file '/not/a/valid/path' #CHECKERR: warning: An error occurred while redirecting file '/not/a/valid/path'
#CHECKERR: open: No such file or directory #CHECKERR: warning: Path '/not' does not exist
#CHECK: Not hung #CHECK: Not hung

View file

@ -138,3 +138,7 @@ echo "/bin/echo pipe 12 <&12 12<&-" | source 12<&0
#CHECK: pipe 10 #CHECK: pipe 10
#CHECK: pipe 11 #CHECK: pipe 11
#CHECK: pipe 12 #CHECK: pipe 12
echo foo >/bin/echo/file
#CHECKERR: warning: An error occurred while redirecting file '/bin/echo/file'
#CHECKERR: warning: Path '/bin/echo' is not a directory

View file

@ -37,6 +37,6 @@ expect_prompt("jobs: There are no jobs", unmatched="Should be no jobs")
# Check that this weird invalid double-redirection doesn't crash fish. # Check that this weird invalid double-redirection doesn't crash fish.
sendline("cat | cat </non/existent/file") sendline("cat | cat </non/existent/file")
expect_str("warning: An error occurred while redirecting file '/non/existent/file'") expect_str("warning: An error occurred while redirecting file '/non/existent/file'")
expect_str("open: No such file or directory") expect_str("warning: Path '/non' does not exist")
send("\x04") send("\x04")
expect_prompt() expect_prompt()