From 5f7a3972114b00919ecd730f1295218daae01289 Mon Sep 17 00:00:00 2001 From: Christopher Serr Date: Mon, 18 Mar 2024 21:02:00 +0100 Subject: [PATCH] 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`. --- Cargo.toml | 2 +- src/common.rs | 6 +-- src/unix/apple/app_store/process.rs | 6 +-- src/unix/apple/macos/process.rs | 58 ++++++++++++++--------------- src/unix/freebsd/process.rs | 8 ++-- src/unix/freebsd/utils.rs | 15 +++++--- src/unix/linux/process.rs | 19 +++++----- src/unknown/process.rs | 6 +-- src/windows/process.rs | 34 +++++++---------- tests/process.rs | 5 ++- 10 files changed, 77 insertions(+), 82 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dea4cbc25..439feafc4 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 7659129b7..b88a8bfbd 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 bac4798cd..e23c7498d 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 71c21b08a..7712166b9 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, + pub(crate) cmd: Vec, pub(crate) exe: Option, pid: Pid, parent: Option, - pub(crate) environ: Vec, + pub(crate) environ: Vec, cwd: Option, pub(crate) root: Option, 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, + cmd: &mut Vec, 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, mut data: &[u8]) { +fn get_environ(environ: &mut Vec, 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 bc9fa3a57..9f213caee 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, + pub(crate) cmd: Vec, pub(crate) exe: Option, pub(crate) pid: Pid, parent: Option, - pub(crate) environ: Vec, + pub(crate) environ: Vec, pub(crate) cwd: Option, pub(crate) root: Option, 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 555f32882..04cf8d276 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 { +pub(crate) unsafe fn get_sys_value_str( + mib: &[c_int], + buf: &mut [libc::c_char], +) -> Option { 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::()]) + Some(c_buf_to_os_string( + &buf[..len / mem::size_of::()], + )) } pub(crate) unsafe fn get_sys_value_by_name(name: &[u8], value: &mut T) -> bool { @@ -248,7 +253,7 @@ pub(crate) fn get_system_info(mib: &[c_int], default: Option<&str>) -> Option Vec { +pub(crate) unsafe fn from_cstr_array(ptr: *const *const c_char) -> Vec { if ptr.is_null() { return Vec::new(); } @@ -267,9 +272,7 @@ pub(crate) unsafe fn from_cstr_array(ptr: *const *const c_char) -> Vec { 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 d45f4b27b..ef324acd7 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, + pub(crate) cmd: Vec, pub(crate) exe: Option, pub(crate) pid: Pid, parent: Option, - pub(crate) environ: Vec, + pub(crate) environ: Vec, pub(crate) cwd: Option, pub(crate) root: Option, 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 { +fn copy_from_file(entry: &Path) -> Vec { 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 { 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 bca71e07f..00d8e4b4a 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 e84f84c7a..b95b1cbe8 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, + cmd: Vec, exe: Option, pid: Pid, user_id: Option, - environ: Vec, + environ: Vec, cwd: Option, root: Option, 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 { +unsafe fn get_cmdline_from_buffer(buffer: PCWSTR) -> Vec { // Get argc and argv from the command line let mut argc = MaybeUninit::::uninit(); let argv_p = CommandLineToArgvW(buffer, argc.as_mut_ptr()); @@ -771,7 +771,7 @@ unsafe fn get_cmdline_from_buffer(buffer: PCWSTR) -> Vec { 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( } } -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(params: &T, handle: HANDLE) -> Vec { +fn get_cmd_line_old(params: &T, handle: HANDLE) -> Vec { 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(params: &T, handle: HANDLE) -> } #[allow(clippy::cast_ptr_alignment)] -fn get_cmd_line_new(handle: HANDLE) -> Vec { +fn get_cmd_line_new(handle: HANDLE) -> Vec { unsafe { if let Some(buffer) = ph_query_process_variable_size(handle, ProcessCommandLineInformation) { @@ -1055,7 +1053,7 @@ fn get_cmd_line( params: &T, handle: HANDLE, refresh_kind: ProcessRefreshKind, - cmd_line: &mut Vec, + cmd_line: &mut Vec, ) { if !refresh_kind.cmd().needs_update(|| cmd_line.is_empty()) { return; @@ -1071,7 +1069,7 @@ fn get_proc_env( params: &T, handle: HANDLE, refresh_kind: ProcessRefreshKind, - environ: &mut Vec, + environ: &mut Vec, ) { if !refresh_kind.environ().needs_update(|| environ.is_empty()) { return; @@ -1085,11 +1083,7 @@ fn get_proc_env( 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; diff --git a/tests/process.rs b/tests/process.rs index 54d16d508..132f8dc78 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!"); }