Merge pull request #8371 from drinkcat/df-osstr

df: Move to using `OsString`

Author: cakebaker

Cargo.lock

@@ -101,7 +101,7 @@ impl OrderChecker {
             return true;
         }
 
-        let is_ordered = current_line >= &self.last_line;
+        let is_ordered = *current_line >= *self.last_line;
         if !is_ordered && !self.has_error {
             eprintln!(
                 "{}",

fuzz/Cargo.lock

@@ -20,6 +20,7 @@ use uucore::{format_usage, show};
 use clap::{Arg, ArgAction, ArgMatches, Command, parser::ValueSource};
 
 use std::ffi::OsString;
+use std::io::stdout;
 use std::path::Path;
 use thiserror::Error;
 
@@ -431,7 +432,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
 
     let opt = Options::from(&matches).map_err(DfError::OptionsError)?;
     // Get the list of filesystems to display in the output table.
-    let filesystems: Vec<Filesystem> = match matches.get_many::<String>(OPT_PATHS) {
+    let filesystems: Vec<Filesystem> = match matches.get_many::<OsString>(OPT_PATHS) {
         None => {
             let filesystems = get_all_filesystems(&opt).map_err(|e| {
                 let context = get_message("df-error-cannot-read-table-of-mounted-filesystems");
@@ -464,7 +465,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         }
     };
 
-    println!("{}", Table::new(&opt, filesystems));
+    Table::new(&opt, filesystems).write_to(&mut stdout())?;
 
     Ok(())
 }
@@ -611,6 +612,7 @@ pub fn uu_app() -> Command {
         .arg(
             Arg::new(OPT_PATHS)
                 .action(ArgAction::Append)
+                .value_parser(ValueParser::os_string())
                 .value_hint(clap::ValueHint::AnyPath),
         )
 }
@@ -629,9 +631,9 @@ mod tests {
                 dev_id: String::new(),
                 dev_name: String::from(dev_name),
                 fs_type: String::new(),
-                mount_dir: String::from(mount_dir),
+                mount_dir: mount_dir.into(),
                 mount_option: String::new(),
-                mount_root: String::from(mount_root),
+                mount_root: mount_root.into(),
                 remote: false,
                 dummy: false,
             }
@@ -679,9 +681,9 @@ mod tests {
                 dev_id: String::from(dev_id),
                 dev_name: String::new(),
                 fs_type: String::new(),
-                mount_dir: String::from(mount_dir),
+                mount_dir: mount_dir.into(),
                 mount_option: String::new(),
-                mount_root: String::new(),
+                mount_root: "/".into(),
                 remote: false,
                 dummy: false,
             }
@@ -724,9 +726,9 @@ mod tests {
                 dev_id: String::new(),
                 dev_name: String::new(),
                 fs_type: String::from(fs_type),
-                mount_dir: String::from(mount_dir),
+                mount_dir: mount_dir.into(),
                 mount_option: String::new(),
-                mount_root: String::new(),
+                mount_root: "/".into(),
                 remote,
                 dummy,
             }

src/uu/comm/src/comm.rs

@@ -8,7 +8,7 @@
 //! filesystem mounted at a particular directory. It also includes
 //! information on amount of space available and amount of space used.
 // spell-checker:ignore canonicalized
-use std::path::Path;
+use std::{ffi::OsString, path::Path};
 
 #[cfg(unix)]
 use uucore::fsext::statfs;
@@ -28,7 +28,7 @@ pub(crate) struct Filesystem {
     /// When invoking `df` with a positional argument, it displays
     /// usage information for the filesystem that contains the given
     /// file. If given, this field contains that filename.
-    pub file: Option<String>,
+    pub file: Option<OsString>,
 
     /// Information about the mounted device, mount directory, and related options.
     pub mount_info: MountInfo,
@@ -123,22 +123,22 @@ where
 
 impl Filesystem {
     // TODO: resolve uuid in `mount_info.dev_name` if exists
-    pub(crate) fn new(mount_info: MountInfo, file: Option<String>) -> Option<Self> {
+    pub(crate) fn new(mount_info: MountInfo, file: Option<OsString>) -> Option<Self> {
         let _stat_path = if mount_info.mount_dir.is_empty() {
             #[cfg(unix)]
             {
-                mount_info.dev_name.clone()
+                mount_info.dev_name.clone().into()
             }
             #[cfg(windows)]
             {
                 // On windows, we expect the volume id
-                mount_info.dev_id.clone()
+                mount_info.dev_id.clone().into()
             }
         } else {
             mount_info.mount_dir.clone()
         };
         #[cfg(unix)]
-        let usage = FsUsage::new(statfs(_stat_path).ok()?);
+        let usage = FsUsage::new(statfs(&_stat_path).ok()?);
         #[cfg(windows)]
         let usage = FsUsage::new(Path::new(&_stat_path)).ok()?;
         Some(Self {
@@ -154,7 +154,7 @@ impl Filesystem {
     pub(crate) fn from_mount(
         mounts: &[MountInfo],
         mount: &MountInfo,
-        file: Option<String>,
+        file: Option<OsString>,
     ) -> Result<Self, FsError> {
         if is_over_mounted(mounts, mount) {
             Err(FsError::OverMounted)
@@ -165,7 +165,7 @@ impl Filesystem {
 
     /// Find and create the filesystem from the given mount.
     #[cfg(windows)]
-    pub(crate) fn from_mount(mount: &MountInfo, file: Option<String>) -> Result<Self, FsError> {
+    pub(crate) fn from_mount(mount: &MountInfo, file: Option<OsString>) -> Result<Self, FsError> {
         Self::new(mount.clone(), file).ok_or(FsError::MountMissing)
     }
 
@@ -189,7 +189,7 @@ impl Filesystem {
     where
         P: AsRef<Path>,
     {
-        let file = path.as_ref().display().to_string();
+        let file = path.as_ref().as_os_str().to_owned();
         let canonicalize = true;
 
         let result = mount_info_from_path(mounts, path, canonicalize);
@@ -205,6 +205,8 @@ mod tests {
 
     mod mount_info_from_path {
 
+        use std::ffi::OsString;
+
         use uucore::fsext::MountInfo;
 
         use crate::filesystem::{FsError, mount_info_from_path};
@@ -215,9 +217,9 @@ mod tests {
                 dev_id: String::default(),
                 dev_name: String::default(),
                 fs_type: String::default(),
-                mount_dir: String::from(mount_dir),
+                mount_dir: OsString::from(mount_dir),
                 mount_option: String::default(),
-                mount_root: String::default(),
+                mount_root: OsString::default(),
                 remote: Default::default(),
                 dummy: Default::default(),
             }
@@ -312,6 +314,8 @@ mod tests {
 
     #[cfg(not(windows))]
     mod over_mount {
+        use std::ffi::OsString;
+
         use crate::filesystem::{Filesystem, FsError, is_over_mounted};
         use uucore::fsext::MountInfo;
 
@@ -320,9 +324,9 @@ mod tests {
                 dev_id: String::default(),
                 dev_name: dev_name.map(String::from).unwrap_or_default(),
                 fs_type: String::default(),
-                mount_dir: String::from(mount_dir),
+                mount_dir: OsString::from(mount_dir),
                 mount_option: String::default(),
-                mount_root: String::default(),
+                mount_root: OsString::default(),
                 remote: Default::default(),
                 dummy: Default::default(),
             }