Rework tail plateform module to tackle #2873 #3706
Conversation
anastygnome
force-pushed
the
tail_refactor
branch
from
c3acea5
to
0e106d3
Compare
anastygnome
force-pushed
the
tail_refactor
branch
11 times, most recently
from
cac7297
to
00dd325
Compare
anastygnome
force-pushed
the
tail_refactor
branch
3 times, most recently
from
b8217a4
to
a04b29e
Compare
|
@sylvestre trying to use isatty on stdin to do the stdin in is_pipe_or_fifo check results in a not implemented error on occasions. I had to use fstat is this a known phenomenon or not ? @tertsdiepraam |
anastygnome
force-pushed
the
tail_refactor
branch
from
a04b29e
to
3de3491
Compare
|
I haven't seen that before. Which |
|
The problem is at the libc level, I've tried all three When piping in /dev/null (or stdin was closed), the problem occurred. |
|
Interesting, I can't check for myself right now, but what was the exact error? |
It panics with a not yet implemented, which I don't get as we're using it for other utils. @sylvestre failures are unrelated, so we can merge this, I'll change it to use atty when this will be sorted out |
| // IFCHR means the file (stdin) is a character input device, which is the case of a terminal. | ||
| // We just need to check if stdin is not a character device here, because we are not interested | ||
| // in the type of stdin itself. | ||
| fstat(libc::STDIN_FILENO).map_or(false, |file| file.st_mode as libc::mode_t & S_IFCHR == 0) |
There was a problem hiding this comment.
Can you please elaborate why you switched S_IFIFO and S_IFSOCK with S_IFCHR ?
There was a problem hiding this comment.
when stdin is connected to a terminal, it symlinks to /dev/tty
which according to the tty manpage
/dev/tty is a character file with major number 5 and minor number 0, usually with mode 0666 and ownership root:tty.
So we just need to check if stdin is not describing (fd) a character file
@tertsdiepraam @jhscheer This error was due to the not inplemented handling of block devices, it turns out isatty mistakenly reports that stdin is a terminal because rust reopens stdin as /dev/null which is a block device @jhscheer here's an in-depth post on how to implement this case, which I put here for reference. |
anastygnome
force-pushed
the
tail_refactor
branch
2 times, most recently
from
ce65376
to
d9290d3
Compare
Optimize tail plateform module using the libc::stdin fd constant. Commenting out `is_bad_symlink` as uutils#2873 will not be fixed for the time being.
sylvestre
force-pushed
the
tail_refactor
branch
from
d9290d3
to
1596f80
Compare
For the moment, these are just performance improvements (using the right name, not checking if the fd is negative as that can't be in rust, using the libc value)
The goal of this is to tackle #2873 more easily later on.