Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use OsString for Process Args and Env Vars #1227

Merged
merged 1 commit into from
Mar 18, 2024
Merged

Use OsString for Process Args and Env Vars #1227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -95,4 +96,3 @@ tempfile = "3.9"

[dev-dependencies]
serde_json = "1.0" # Used in documentation tests.
bstr = "1.9.0" # Used in example
6 changes: 3 additions & 3 deletions src/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -918,7 +918,7 @@ impl Process {
/// println!("{:?}", process.cmd());
/// }
/// ```
pub fn cmd(&self) -> &[String] {
pub fn cmd(&self) -> &[OsString] {
self.inner.cmd()
}

Expand Down Expand Up @@ -972,7 +972,7 @@ impl Process {
/// println!("{:?}", process.environ());
/// }
/// ```
pub fn environ(&self) -> &[String] {
pub fn environ(&self) -> &[OsString] {
self.inner.environ()
}

Expand Down
6 changes: 3 additions & 3 deletions src/unix/apple/app_store/process.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -16,7 +16,7 @@ impl ProcessInner {
OsStr::new("")
}

pub(crate) fn cmd(&self) -> &[String] {
pub(crate) fn cmd(&self) -> &[OsString] {
&[]
}

Expand All @@ -28,7 +28,7 @@ impl ProcessInner {
Pid(0)
}

pub(crate) fn environ(&self) -> &[String] {
pub(crate) fn environ(&self) -> &[OsString] {
&[]
}

Expand Down
58 changes: 27 additions & 31 deletions src/unix/apple/macos/process.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -118,7 +118,7 @@ impl ProcessInner {
&self.name
}

pub(crate) fn cmd(&self) -> &[String] {
pub(crate) fn cmd(&self) -> &[OsString] {
&self.cmd
}

Expand All @@ -130,7 +130,7 @@ impl ProcessInner {
self.pid
}

pub(crate) fn environ(&self) -> &[String] {
pub(crate) fn environ(&self) -> &[OsString] {
&self.environ
}

Expand Down Expand Up @@ -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,
Expand All @@ -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..];
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/unix/freebsd/process.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -77,7 +77,7 @@ impl ProcessInner {
&self.name
}

pub(crate) fn cmd(&self) -> &[String] {
pub(crate) fn cmd(&self) -> &[OsString] {
&self.cmd
}

Expand All @@ -89,7 +89,7 @@ impl ProcessInner {
self.pid
}

pub(crate) fn environ(&self) -> &[String] {
pub(crate) fn environ(&self) -> &[OsString] {
&self.environ
}

Expand Down
15 changes: 9 additions & 6 deletions src/unix/freebsd/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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 {
Expand Down Expand Up @@ -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();
}
Expand All @@ -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
}
Expand Down
19 changes: 10 additions & 9 deletions src/unix/linux/process.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -170,7 +171,7 @@ impl ProcessInner {
&self.name
}

pub(crate) fn cmd(&self) -> &[String] {
pub(crate) fn cmd(&self) -> &[OsString] {
&self.cmd
}

Expand All @@ -182,7 +183,7 @@ impl ProcessInner {
self.pid
}

pub(crate) fn environ(&self) -> &[String] {
pub(crate) fn environ(&self) -> &[OsString] {
&self.environ
}

Expand Down Expand Up @@ -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()?);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use to_str_lossy here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For lossiness, the Unicode replacement character (�) is used, but that never results in a validly parsed usize, so might as well skip directly to returning an error, instead of allocating and then erroring anyway.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. Thanks for the explanations!


let tasks_dir = Path::join(&entry, "task");
let tasks = if tasks_dir.is_dir() {
Expand Down Expand Up @@ -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);
Expand All @@ -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..];
}
Expand Down
6 changes: 3 additions & 3 deletions src/unknown/process.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -26,7 +26,7 @@ impl ProcessInner {
OsStr::new("")
}

pub(crate) fn cmd(&self) -> &[String] {
pub(crate) fn cmd(&self) -> &[OsString] {
&[]
}

Expand All @@ -38,7 +38,7 @@ impl ProcessInner {
self.pid
}

pub(crate) fn environ(&self) -> &[String] {
pub(crate) fn environ(&self) -> &[OsString] {
&[]
}

Expand Down