tail: Cleanup code, fix clippy warnings and add documentation

src/uu/tail/src/args.rs

@@ -5,6 +5,8 @@
 
 // spell-checker:ignore (ToDO) kqueue Signum fundu
 
+//! Parse the arguments passed to `tail` into a [`Settings`] struct.
+
 use crate::paths::Input;
 use crate::{parse, platform, Quotable};
 use clap::crate_version;
@@ -41,6 +43,7 @@ pub mod options {
  pub static PRESUME_INPUT_PIPE: &str = "-presume-input-pipe"; // NOTE: three hyphens is correct
 }
 
+/// Represent a `u64` with sign. `0` is a special value and can be negative or positive.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum Signum {
  Negative(u64),
@@ -49,8 +52,12 @@ pub enum Signum {
  MinusZero,
 }
 
+/// The tail operation mode. Can be either `Lines` or `Bytes`.
+///
+/// `Lines` for the `-n` option and `Bytes` for the `-c` option.
 #[derive(Debug, PartialEq, Eq)]
 pub enum FilterMode {
+ /// Mode for bytes.
  Bytes(Signum),
 
  /// Mode for lines delimited by delimiter as u8
@@ -116,29 +123,56 @@ impl Default for FilterMode {
  }
 }
 
+/// The `tail` follow mode given by the `--follow` flag.
+///
+/// Can bei either `Descriptor` (`--follow=descriptor`) which is the default or `Name`
+/// (`--follow=name`)
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub enum FollowMode {
  Descriptor,
  Name,
 }
 
+/// The result returned from [`Settings::verify`].
 #[derive(Debug)]
 pub enum VerificationResult {
+ /// Returned if [`Settings::verify`] has not detected any misconfiguration of the command line
+ /// arguments
  Ok,
+
+ // Returned if one of the file arguments passed to `tail` is the stdin file descriptor (`-`) and
+ // `--follow=name` was given.
  CannotFollowStdinByName,
+
+ /// Returned if tail will not output anything because of the configuration passed to `tail`.
  NoOutput,
 }
 
+/// Store the configuration of `tail` parsed from the command line arguments (and defaults).
+///
+/// This struct is designed to store the initial values given from user provided command line
+/// arguments if present or sane defaults if not. The fields of `Settings` and `Settings` itself
+/// should be seen as constants. Please do not use `Settings` to store fields that need to be
+/// mutable after the initialization of `Settings`.
 #[derive(Debug)]
 pub struct Settings {
+ /// `--follow`, `-f` and as part of `-F`
  pub follow: Option<FollowMode>,
+ /// `--max-unchanged-stats`
  pub max_unchanged_stats: u32,
+ /// `--lines`, `-n` or `--bytes`, `-c`
  pub mode: FilterMode,
+ /// `--pid`
  pub pid: platform::Pid,
+ /// `--retry` and as part of `-F`
  pub retry: bool,
+ /// `--sleep-interval`, `-s`
  pub sleep_sec: Duration,
+ /// `--use-polling` (non standard: divergence to gnu's `tail` )
  pub use_polling: bool,
+ /// `--verbose`, `-v` and `--quiet`, `-q`
  pub verbose: bool,
+ /// `---presume-input-pipe`
  pub presume_input_pipe: bool,
  /// `FILE(s)` positional arguments
  pub inputs: Vec<Input>,
@@ -181,7 +215,7 @@ impl Settings {
  settings
  }
 
- pub fn from(matches: &clap::ArgMatches) -> UResult<Self> {
+ pub fn from(matches: &ArgMatches) -> UResult<Self> {
  let mut settings: Self = Self {
  follow: if matches.get_flag(options::FOLLOW_RETRY) {
  Some(FollowMode::Name)
@@ -323,10 +357,11 @@ impl Settings {
  }
  }
 
- /// Verify [`Settings`] and try to find unsolvable misconfigurations of tail originating from
- /// user provided command line arguments. In contrast to [`Settings::check_warnings`] these
- /// misconfigurations usually lead to the immediate exit or abortion of the running `tail`
- /// process.
+ /// Verify the [`Settings`] and try to find unsolvable misconfigurations of tail originating
+ /// from user provided command line arguments.
+ ///
+ /// In contrast to [`Settings::check_warnings`] these misconfigurations usually lead to the
+ /// immediate exit or abortion of the running `tail` process.
  pub fn verify(&self) -> VerificationResult {
  // Mimic GNU's tail for `tail -F`
  if self.inputs.iter().any(|i| i.is_stdin()) && self.follow == Some(FollowMode::Name) {

src/uu/tail/src/follow/files.rs

@@ -134,6 +134,7 @@ impl FileHandling {
  };
  }
 
+ // TODO: return io::Result and handle io::Errors
  /// Read new data from `path` and print it to stdout
  pub fn tail_file(&mut self, path: &Path, verbose: bool) -> UResult<bool> {
  let mut chunks = BytesChunkBuffer::new(u64::MAX);
@@ -162,18 +163,19 @@ impl FileHandling {
  /// Decide if printing `path` needs a header based on when it was last printed
  pub fn needs_header(&self, path: &Path, verbose: bool) -> bool {
  if verbose {
- if let Some(ref last) = self.last {
- return !last.eq(&path);
+ return if let Some(ref last) = self.last {
+ !last.eq(&path)
  } else {
- return true;
- }
+ true
+ };
  }
  false
  }
 }
 
 /// Data structure to keep a handle on the BufReader, Metadata
 /// and the display_name (header_name) of files that are being followed.
+/// FIXME: store File instead of a Buffered Reader
 pub struct PathData {
  pub reader: Option<Box<dyn BufRead>>,
  pub metadata: Option<Metadata>,

src/uu/tail/src/platform/mod.rs

@@ -9,12 +9,7 @@
  */
 
 #[cfg(unix)]
-pub use self::unix::{
- //stdin_is_bad_fd, stdin_is_pipe_or_fifo, supports_pid_checks, Pid, ProcessChecker,
- supports_pid_checks,
- Pid,
- ProcessChecker,
-};
+pub use self::unix::{supports_pid_checks, Pid, ProcessChecker};
 
 #[cfg(windows)]
 pub use self::windows::{supports_pid_checks, Pid, ProcessChecker};

src/uu/tail/src/platform/unix.rs

@@ -16,11 +16,11 @@ use std::io::Error;
 pub type Pid = libc::pid_t;
 
 pub struct ProcessChecker {
- pid: self::Pid,
+ pid: Pid,
 }
 
 impl ProcessChecker {
- pub fn new(process_id: self::Pid) -> Self {
+ pub fn new(process_id: Pid) -> Self {
  Self { pid: process_id }
  }
 
@@ -35,20 +35,11 @@ impl Drop for ProcessChecker {
  fn drop(&mut self) {}
 }
 
-pub fn supports_pid_checks(pid: self::Pid) -> bool {
+pub fn supports_pid_checks(pid: Pid) -> bool {
  unsafe { !(libc::kill(pid, 0) != 0 && get_errno() == libc::ENOSYS) }
 }
 
 #[inline]
 fn get_errno() -> i32 {
  Error::last_os_error().raw_os_error().unwrap()
 }
-
-//pub fn stdin_is_bad_fd() -> bool {
-// FIXME: Detect a closed file descriptor, e.g.: `tail <&-`
-// this is never `true`, even with `<&-` because Rust's stdlib is reopening fds as /dev/null
-// see also: https:/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
-//}

src/uu/tail/src/platform/windows.rs

@@ -19,9 +19,8 @@ pub struct ProcessChecker {
 }
 
 impl ProcessChecker {
- pub fn new(process_id: self::Pid) -> Self {
- #[allow(non_snake_case)]
- let FALSE: BOOL = 0;
+ pub fn new(process_id: Pid) -> Self {
+ const FALSE: BOOL = 0;
  let h = unsafe { OpenProcess(PROCESS_SYNCHRONIZE, FALSE, process_id) };
  Self {
  dead: h == 0,
@@ -50,6 +49,6 @@ impl Drop for ProcessChecker {
  }
 }
 
-pub fn supports_pid_checks(_pid: self::Pid) -> bool {
+pub fn supports_pid_checks(_pid: Pid) -> bool {
  true
 }