Repository: GuillaumeGomez/sysinfo
Pull request #1227 (2024-03-18 20:03:37)

Use `OsString` for Process Args and Env Vars
This continues the "os stringification" on the `Process` type where now not only the process name, but also the arguments and environment variables are also done as `OsString`, mirroring `std`.


Patch:
diff --git a/Cargo.toml b/Cargo.toml
index dea4cbc..439feaf 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -41,6 +41,7 @@ cargo-args = ["-Zbuild-std"]
 rustdoc-args = ["--generate-link-to-definition"]
 
 [dependencies]
+bstr = "1.9.0"
 cfg-if = "1.0"
 memchr = "2.7.1"
 rayon = { version = "^1.8", optional = true }
@@ -95,4 +96,3 @@ tempfile = "3.9"
 
 [dev-dependencies]
 serde_json = "1.0" # Used in documentation tests.
-bstr = "1.9.0" # Used in example
diff --git a/src/common.rs b/src/common.rs
index 7659129..b88a8bf 100644
--- a/src/common.rs
+++ b/src/common.rs
@@ -8,7 +8,7 @@ use crate::{
 use std::cmp::Ordering;
 use std::collections::{HashMap, HashSet};
 use std::convert::TryFrom;
-use std::ffi::OsStr;
+use std::ffi::{OsStr, OsString};
 use std::fmt;
 use std::fmt::Formatter;
 use std::net::IpAddr;
@@ -918,7 +918,7 @@ impl Process {
     ///     println!("{:?}", process.cmd());
     /// }
     /// ```
-    pub fn cmd(&self) -> &[String] {
+    pub fn cmd(&self) -> &[OsString] {
         self.inner.cmd()
     }
 
@@ -972,7 +972,7 @@ impl Process {
     ///     println!("{:?}", process.environ());
     /// }
     /// ```
-    pub fn environ(&self) -> &[String] {
+    pub fn environ(&self) -> &[OsString] {
         self.inner.environ()
     }
 
diff --git a/src/unix/apple/app_store/process.rs b/src/unix/apple/app_store/process.rs
index bac4798..e23c749 100644
--- a/src/unix/apple/app_store/process.rs
+++ b/src/unix/apple/app_store/process.rs
@@ -1,6 +1,6 @@
 // Take a look at the license at the top of the repository in the LICENSE file.
 
-use std::ffi::OsStr;
+use std::ffi::{OsStr, OsString};
 use std::path::Path;
 
 use crate::{DiskUsage, Gid, Pid, ProcessStatus, Signal, Uid};
@@ -16,7 +16,7 @@ impl ProcessInner {
         OsStr::new("")
     }
 
-    pub(crate) fn cmd(&self) -> &[String] {
+    pub(crate) fn cmd(&self) -> &[OsString] {
         &[]
     }
 
@@ -28,7 +28,7 @@ impl ProcessInner {
         Pid(0)
     }
 
-    pub(crate) fn environ(&self) -> &[String] {
+    pub(crate) fn environ(&self) -> &[OsString] {
         &[]
     }
 
diff --git a/src/unix/apple/macos/process.rs b/src/unix/apple/macos/process.rs
index 71c21b0..7712166 100644
--- a/src/unix/apple/macos/process.rs
+++ b/src/unix/apple/macos/process.rs
@@ -15,11 +15,11 @@ use crate::unix::utils::cstr_to_rust_with_size;
 
 pub(crate) struct ProcessInner {
     pub(crate) name: OsString,
-    pub(crate) cmd: Vec<String>,
+    pub(crate) cmd: Vec<OsString>,
     pub(crate) exe: Option<PathBuf>,
     pid: Pid,
     parent: Option<Pid>,
-    pub(crate) environ: Vec<String>,
+    pub(crate) environ: Vec<OsString>,
     cwd: Option<PathBuf>,
     pub(crate) root: Option<PathBuf>,
     pub(crate) memory: u64,
@@ -118,7 +118,7 @@ impl ProcessInner {
         &self.name
     }
 
-    pub(crate) fn cmd(&self) -> &[String] {
+    pub(crate) fn cmd(&self) -> &[OsString] {
         &self.cmd
     }
 
@@ -130,7 +130,7 @@ impl ProcessInner {
         self.pid
     }
 
-    pub(crate) fn environ(&self) -> &[String] {
+    pub(crate) fn environ(&self) -> &[OsString] {
         &self.environ
     }
 
@@ -571,7 +571,7 @@ fn get_exe(data: &[u8]) -> (&Path, &[u8]) {
 }
 
 fn get_arguments<'a>(
-    cmd: &mut Vec<String>,
+    cmd: &mut Vec<OsString>,
     mut data: &'a [u8],
     mut n_args: c_int,
     refresh_cmd: bool,
@@ -587,42 +587,38 @@ fn get_arguments<'a>(
         data = &data[1..];
     }
 
-    unsafe {
-        while n_args > 0 && !data.is_empty() {
-            let pos = data.iter().position(|c| *c == 0).unwrap_or(data.len());
-            let arg = std::str::from_utf8_unchecked(&data[..pos]);
-            if !arg.is_empty() && refresh_cmd {
-                cmd.push(arg.to_string());
-            }
-            data = &data[pos..];
-            while data.first() == Some(&0) {
-                data = &data[1..];
-            }
-            n_args -= 1;
+    while n_args > 0 && !data.is_empty() {
+        let pos = data.iter().position(|c| *c == 0).unwrap_or(data.len());
+        let arg = &data[..pos];
+        if !arg.is_empty() && refresh_cmd {
+            cmd.push(OsStr::from_bytes(arg).to_os_string());
+        }
+        data = &data[pos..];
+        while data.first() == Some(&0) {
+            data = &data[1..];
         }
-        data
+        n_args -= 1;
     }
+    data
 }
 
-fn get_environ(environ: &mut Vec<String>, mut data: &[u8]) {
+fn get_environ(environ: &mut Vec<OsString>, mut data: &[u8]) {
     environ.clear();
 
     while data.first() == Some(&0) {
         data = &data[1..];
     }
 
-    unsafe {
-        while !data.is_empty() {
-            let pos = data.iter().position(|c| *c == 0).unwrap_or(data.len());
-            let arg = std::str::from_utf8_unchecked(&data[..pos]);
-            if arg.is_empty() {
-                return;
-            }
-            environ.push(arg.to_string());
-            data = &data[pos..];
-            while data.first() == Some(&0) {
-                data = &data[1..];
-            }
+    while !data.is_empty() {
+        let pos = data.iter().position(|c| *c == 0).unwrap_or(data.len());
+        let arg = &data[..pos];
+        if arg.is_empty() {
+            return;
+        }
+        environ.push(OsStr::from_bytes(arg).to_os_string());
+        data = &data[pos..];
+        while data.first() == Some(&0) {
+            data = &data[1..];
         }
     }
 }
diff --git a/src/unix/freebsd/process.rs b/src/unix/freebsd/process.rs
index bc9fa3a..9f213ca 100644
--- a/src/unix/freebsd/process.rs
+++ b/src/unix/freebsd/process.rs
@@ -43,11 +43,11 @@ impl fmt::Display for ProcessStatus {
 
 pub(crate) struct ProcessInner {
     pub(crate) name: OsString,
-    pub(crate) cmd: Vec<String>,
+    pub(crate) cmd: Vec<OsString>,
     pub(crate) exe: Option<PathBuf>,
     pub(crate) pid: Pid,
     parent: Option<Pid>,
-    pub(crate) environ: Vec<String>,
+    pub(crate) environ: Vec<OsString>,
     pub(crate) cwd: Option<PathBuf>,
     pub(crate) root: Option<PathBuf>,
     pub(crate) memory: u64,
@@ -77,7 +77,7 @@ impl ProcessInner {
         &self.name
     }
 
-    pub(crate) fn cmd(&self) -> &[String] {
+    pub(crate) fn cmd(&self) -> &[OsString] {
         &self.cmd
     }
 
@@ -89,7 +89,7 @@ impl ProcessInner {
         self.pid
     }
 
-    pub(crate) fn environ(&self) -> &[String] {
+    pub(crate) fn environ(&self) -> &[OsString] {
         &self.environ
     }
 
diff --git a/src/unix/freebsd/utils.rs b/src/unix/freebsd/utils.rs
index 555f328..04cf8d2 100644
--- a/src/unix/freebsd/utils.rs
+++ b/src/unix/freebsd/utils.rs
@@ -143,7 +143,10 @@ pub(crate) fn c_buf_to_os_string(buf: &[libc::c_char]) -> OsString {
     c_buf_to_os_str(buf).to_owned()
 }
 
-pub(crate) unsafe fn get_sys_value_str(mib: &[c_int], buf: &mut [libc::c_char]) -> Option<String> {
+pub(crate) unsafe fn get_sys_value_str(
+    mib: &[c_int],
+    buf: &mut [libc::c_char],
+) -> Option<OsString> {
     let mut len = mem::size_of_val(buf) as libc::size_t;
     if libc::sysctl(
         mib.as_ptr(),
@@ -156,7 +159,9 @@ pub(crate) unsafe fn get_sys_value_str(mib: &[c_int], buf: &mut [libc::c_char])
     {
         return None;
     }
-    c_buf_to_utf8_string(&buf[..len / mem::size_of::<libc::c_char>()])
+    Some(c_buf_to_os_string(
+        &buf[..len / mem::size_of::<libc::c_char>()],
+    ))
 }
 
 pub(crate) unsafe fn get_sys_value_by_name<T: Sized>(name: &[u8], value: &mut T) -> bool {
@@ -248,7 +253,7 @@ pub(crate) fn get_system_info(mib: &[c_int], default: Option<&str>) -> Option<St
     }
 }
 
-pub(crate) unsafe fn from_cstr_array(ptr: *const *const c_char) -> Vec<String> {
+pub(crate) unsafe fn from_cstr_array(ptr: *const *const c_char) -> Vec<OsString> {
     if ptr.is_null() {
         return Vec::new();
     }
@@ -267,9 +272,7 @@ pub(crate) unsafe fn from_cstr_array(ptr: *const *const c_char) -> Vec<String> {
 
     for pos in 0..max {
         let p = ptr.add(pos);
-        if let Ok(s) = CStr::from_ptr(*p).to_str() {
-            ret.push(s.to_owned());
-        }
+        ret.push(OsStr::from_bytes(CStr::from_ptr(*p).to_bytes()).to_os_string());
     }
     ret
 }
diff --git a/src/unix/linux/process.rs b/src/unix/linux/process.rs
index d45f4b2..ef324ac 100644
--- a/src/unix/linux/process.rs
+++ b/src/unix/linux/process.rs
@@ -10,6 +10,7 @@ use std::os::unix::ffi::OsStrExt;
 use std::path::{Path, PathBuf};
 use std::str::{self, FromStr};
 
+use bstr::ByteSlice;
 use libc::{c_ulong, gid_t, kill, uid_t};
 
 use crate::sys::system::SystemInfo;
@@ -90,11 +91,11 @@ enum ProcIndex {
 
 pub(crate) struct ProcessInner {
     pub(crate) name: OsString,
-    pub(crate) cmd: Vec<String>,
+    pub(crate) cmd: Vec<OsString>,
     pub(crate) exe: Option<PathBuf>,
     pub(crate) pid: Pid,
     parent: Option<Pid>,
-    pub(crate) environ: Vec<String>,
+    pub(crate) environ: Vec<OsString>,
     pub(crate) cwd: Option<PathBuf>,
     pub(crate) root: Option<PathBuf>,
     pub(crate) memory: u64,
@@ -170,7 +171,7 @@ impl ProcessInner {
         &self.name
     }
 
-    pub(crate) fn cmd(&self) -> &[String] {
+    pub(crate) fn cmd(&self) -> &[OsString] {
         &self.cmd
     }
 
@@ -182,7 +183,7 @@ impl ProcessInner {
         self.pid
     }
 
-    pub(crate) fn environ(&self) -> &[String] {
+    pub(crate) fn environ(&self) -> &[OsString] {
         &self.environ
     }
 
@@ -661,7 +662,7 @@ fn get_all_pid_entries(
     if !entry.is_dir() {
         return None;
     }
-    let pid = Pid::from(usize::from_str(&name.to_string_lossy()).ok()?);
+    let pid = Pid::from(usize::from_str(name.to_str()?).ok()?);
 
     let tasks_dir = Path::join(&entry, "task");
     let tasks = if tasks_dir.is_dir() {
@@ -778,7 +779,7 @@ pub(crate) fn refresh_procs(
     true
 }
 
-fn copy_from_file(entry: &Path) -> Vec<String> {
+fn copy_from_file(entry: &Path) -> Vec<OsString> {
     match File::open(entry) {
         Ok(mut f) => {
             let mut data = Vec::with_capacity(16_384);
@@ -790,9 +791,9 @@ fn copy_from_file(entry: &Path) -> Vec<String> {
                 let mut out = Vec::with_capacity(10);
                 let mut data = data.as_slice();
                 while let Some(pos) = data.iter().position(|c| *c == 0) {
-                    match str::from_utf8(&data[..pos]).map(|s| s.trim()) {
-                        Ok(s) if !s.is_empty() => out.push(s.to_string()),
-                        _ => {}
+                    let s = &data[..pos].trim();
+                    if !s.is_empty() {
+                        out.push(OsStr::from_bytes(s).to_os_string());
                     }
                     data = &data[pos + 1..];
                 }
diff --git a/src/unknown/process.rs b/src/unknown/process.rs
index bca71e0..00d8e4b 100644
--- a/src/unknown/process.rs
+++ b/src/unknown/process.rs
@@ -2,7 +2,7 @@
 
 use crate::{DiskUsage, Gid, Pid, ProcessStatus, Signal, Uid};
 
-use std::ffi::OsStr;
+use std::ffi::{OsStr, OsString};
 use std::fmt;
 use std::path::Path;
 
@@ -26,7 +26,7 @@ impl ProcessInner {
         OsStr::new("")
     }
 
-    pub(crate) fn cmd(&self) -> &[String] {
+    pub(crate) fn cmd(&self) -> &[OsString] {
         &[]
     }
 
@@ -38,7 +38,7 @@ impl ProcessInner {
         self.pid
     }
 
-    pub(crate) fn environ(&self) -> &[String] {
+    pub(crate) fn environ(&self) -> &[OsString] {
         &[]
     }
 
diff --git a/src/windows/process.rs b/src/windows/process.rs
index e84f84c..b95b1cb 100644
--- a/src/windows/process.rs
+++ b/src/windows/process.rs
@@ -199,11 +199,11 @@ unsafe impl Sync for HandleWrapper {}
 
 pub(crate) struct ProcessInner {
     name: OsString,
-    cmd: Vec<String>,
+    cmd: Vec<OsString>,
     exe: Option<PathBuf>,
     pid: Pid,
     user_id: Option<Uid>,
-    environ: Vec<String>,
+    environ: Vec<OsString>,
     cwd: Option<PathBuf>,
     root: Option<PathBuf>,
     pub(crate) memory: u64,
@@ -550,7 +550,7 @@ impl ProcessInner {
         &self.name
     }
 
-    pub(crate) fn cmd(&self) -> &[String] {
+    pub(crate) fn cmd(&self) -> &[OsString] {
         &self.cmd
     }
 
@@ -562,7 +562,7 @@ impl ProcessInner {
         self.pid
     }
 
-    pub(crate) fn environ(&self) -> &[String] {
+    pub(crate) fn environ(&self) -> &[OsString] {
         &self.environ
     }
 
@@ -759,7 +759,7 @@ unsafe fn ph_query_process_variable_size(
     Some(buffer)
 }
 
-unsafe fn get_cmdline_from_buffer(buffer: PCWSTR) -> Vec<String> {
+unsafe fn get_cmdline_from_buffer(buffer: PCWSTR) -> Vec<OsString> {
     // Get argc and argv from the command line
     let mut argc = MaybeUninit::<i32>::uninit();
     let argv_p = CommandLineToArgvW(buffer, argc.as_mut_ptr());
@@ -771,7 +771,7 @@ unsafe fn get_cmdline_from_buffer(buffer: PCWSTR) -> Vec<String> {
 
     let mut res = Vec::new();
     for arg in argv {
-        res.push(String::from_utf16_lossy(arg.as_wide()));
+        res.push(OsString::from_wide(arg.as_wide()));
     }
 
     let _err = LocalFree(HLOCAL(argv_p as _));
@@ -1018,16 +1018,14 @@ fn get_cwd_and_root<T: RtlUserProcessParameters>(
     }
 }
 
-unsafe fn null_terminated_wchar_to_string(slice: &[u16]) -> String {
+unsafe fn null_terminated_wchar_to_string(slice: &[u16]) -> OsString {
     match slice.iter().position(|&x| x == 0) {
-        Some(pos) => OsString::from_wide(&slice[..pos])
-            .to_string_lossy()
-            .into_owned(),
-        None => OsString::from_wide(slice).to_string_lossy().into_owned(),
+        Some(pos) => OsString::from_wide(&slice[..pos]),
+        None => OsString::from_wide(slice),
     }
 }
 
-fn get_cmd_line_old<T: RtlUserProcessParameters>(params: &T, handle: HANDLE) -> Vec<String> {
+fn get_cmd_line_old<T: RtlUserProcessParameters>(params: &T, handle: HANDLE) -> Vec<OsString> {
     match params.get_cmdline(handle) {
         Ok(buffer) => unsafe { get_cmdline_from_buffer(PCWSTR::from_raw(buffer.as_ptr())) },
         Err(_e) => {
@@ -1038,7 +1036,7 @@ fn get_cmd_line_old<T: RtlUserProcessParameters>(params: &T, handle: HANDLE) ->
 }
 
 #[allow(clippy::cast_ptr_alignment)]
-fn get_cmd_line_new(handle: HANDLE) -> Vec<String> {
+fn get_cmd_line_new(handle: HANDLE) -> Vec<OsString> {
     unsafe {
         if let Some(buffer) = ph_query_process_variable_size(handle, ProcessCommandLineInformation)
         {
@@ -1055,7 +1053,7 @@ fn get_cmd_line<T: RtlUserProcessParameters>(
     params: &T,
     handle: HANDLE,
     refresh_kind: ProcessRefreshKind,
-    cmd_line: &mut Vec<String>,
+    cmd_line: &mut Vec<OsString>,
 ) {
     if !refresh_kind.cmd().needs_update(|| cmd_line.is_empty()) {
         return;
@@ -1071,7 +1069,7 @@ fn get_proc_env<T: RtlUserProcessParameters>(
     params: &T,
     handle: HANDLE,
     refresh_kind: ProcessRefreshKind,
-    environ: &mut Vec<String>,
+    environ: &mut Vec<OsString>,
 ) {
     if !refresh_kind.environ().needs_update(|| environ.is_empty()) {
         return;
@@ -1085,11 +1083,7 @@ fn get_proc_env<T: RtlUserProcessParameters>(
             while let Some(offset) = raw_env[begin..].iter().position(|&c| c == 0) {
                 let end = begin + offset;
                 if raw_env[begin..end].iter().any(|&c| c == equals) {
-                    environ.push(
-                        OsString::from_wide(&raw_env[begin..end])
-                            .to_string_lossy()
-                            .into_owned(),
-                    );
+                    environ.push(OsString::from_wide(&raw_env[begin..end]));
                     begin = end + 1;
                 } else {
                     break;


Test patch:
diff --git a/tests/process.rs b/tests/process.rs
index 54d16d5..132f8dc 100644
--- a/tests/process.rs
+++ b/tests/process.rs
@@ -1,5 +1,6 @@
 // Take a look at the license at the top of the repository in the LICENSE file.
 
+use bstr::ByteSlice;
 use sysinfo::{Pid, ProcessRefreshKind, System, UpdateKind};
 
 #[test]
@@ -70,7 +71,7 @@ fn test_cmd() {
         if cfg!(target_os = "windows") {
             // Sometimes, we get the full path instead for some reasons... So just in case,
             // we check for the command independently that from the arguments.
-            assert!(process.cmd()[0].contains("waitfor"));
+            assert!(process.cmd()[0].as_encoded_bytes().contains_str("waitfor"));
             assert_eq!(&process.cmd()[1..], &["/t", "3", "CmdSignal"]);
         } else {
             assert_eq!(process.cmd(), &["sleep", "3"]);
@@ -147,7 +148,7 @@ fn test_environ() {
         p.kill().expect("Unable to kill process.");
         assert_eq!(proc_.pid(), pid);
         let env = format!("FOO={big_env}");
-        assert!(proc_.environ().iter().any(|e| *e == env));
+        assert!(proc_.environ().iter().any(|e| *e == *env));
     } else {
         panic!("Process not found!");
     }