Skip to content

Commit

Permalink
Rework tail plateform module in light of uutils#2873
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
anastygnome authored and sylvestre committed Aug 14, 2022
1 parent c8aa5bc commit 1596f80
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 33 deletions.
6 changes: 5 additions & 1 deletion src/uu/tail/src/platform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@

#[cfg(unix)]
pub use self::unix::{
stdin_is_bad_fd, stdin_is_pipe_or_fifo, supports_pid_checks, Pid, ProcessChecker,
//stdin_is_bad_fd, stdin_is_pipe_or_fifo, supports_pid_checks, Pid, ProcessChecker,
stdin_is_pipe_or_fifo,
supports_pid_checks,
Pid,
ProcessChecker,
};

#[cfg(windows)]
Expand Down
43 changes: 16 additions & 27 deletions src/uu/tail/src/platform/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,12 @@
* file that was distributed with this source code.
*/

// spell-checker:ignore (ToDO) stdlib
// spell-checker:ignore (options) GETFD EPERM ENOSYS

use std::io::{stdin, Error};

use std::os::unix::prelude::AsRawFd;
// spell-checker:ignore (ToDO) stdlib, ISCHR, GETFD
// spell-checker:ignore (options) EPERM, ENOSYS

use libc::S_IFCHR;
use nix::sys::stat::fstat;

use libc::{S_IFIFO, S_IFSOCK};
use std::io::Error;

pub type Pid = libc::pid_t;

Expand Down Expand Up @@ -49,26 +45,19 @@ pub fn supports_pid_checks(pid: self::Pid) -> bool {
fn get_errno() -> i32 {
Error::last_os_error().raw_os_error().unwrap()
}

#[inline]
pub fn stdin_is_pipe_or_fifo() -> bool {
let fd = stdin().lock().as_raw_fd();
// GNU tail checks fd >= 0
fd >= 0
&& match fstat(fd) {
Ok(stat) => {
let mode = stat.st_mode as libc::mode_t;
// NOTE: This is probably not the most correct way to check this
(mode & S_IFIFO != 0) || (mode & S_IFSOCK != 0)
}
Err(err) => panic!("{}", err),
}
// 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)
}

//pub fn stdin_is_bad_fd() -> bool {
// FIXME: Detect a closed file descriptor, e.g.: `tail <&-`
pub fn stdin_is_bad_fd() -> bool {
let fd = stdin().as_raw_fd();
// this is never `true`, even with `<&-` because Rust's stdlib is reopening fds as /dev/null
// see also: https://github.com/uutils/coreutils/issues/2873
// (gnu/tests/tail-2/follow-stdin.sh fails because of this)
unsafe { libc::fcntl(fd, libc::F_GETFD) == -1 && get_errno() == libc::EBADF }
}
// this is never `true`, even with `<&-` because Rust's stdlib is reopening fds as /dev/null
// see also: https://github.com/uutils/coreutils/issues/2873
// (gnu/tests/tail-2/follow-stdin.sh fails because of this)
// unsafe { libc::fcntl(fd, libc::F_GETFD) == -1 && get_errno() == libc::EBADF }
//false
//}
13 changes: 8 additions & 5 deletions src/uu/tail/src/tail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// spell-checker:ignore (libs) kqueue
// spell-checker:ignore (acronyms)
// spell-checker:ignore (env/flags)
// spell-checker:ignore (jargon) tailable untailable
// spell-checker:ignore (jargon) tailable untailable stdlib
// spell-checker:ignore (names)
// spell-checker:ignore (shell/tools)
// spell-checker:ignore (misc)
Expand Down Expand Up @@ -1522,13 +1522,16 @@ pub fn stdin_is_pipe_or_fifo() -> bool {
.unwrap_or(false)
}
}

#[inline]
pub fn stdin_is_bad_fd() -> bool {
#[cfg(unix)]
// FIXME : Rust's stdlib is reopening fds as /dev/null
// see also: https://github.com/uutils/coreutils/issues/2873
// (gnu/tests/tail-2/follow-stdin.sh fails because of this)
//#[cfg(unix)]
{
platform::stdin_is_bad_fd()
//platform::stdin_is_bad_fd()
}
#[cfg(not(unix))]
//#[cfg(not(unix))]
false
}

Expand Down

0 comments on commit 1596f80

Please sign in to comment.