From 4992f8896633fb8ca8d89e09f02330cd49395485 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Wed, 7 Aug 2024 11:26:08 +0200 Subject: [PATCH] Replace getrusage with a 64-bit-time_t wrapper Part of #10634 --- src/bin/fish.rs | 18 +++++++----------- src/libc.c | 19 +++++++++++++++++++ src/libc.rs | 26 +++++++++++++++++++++++++- src/nix.rs | 9 +++++---- src/timer.rs | 30 ++++++++++-------------------- 5 files changed, 66 insertions(+), 36 deletions(-) diff --git a/src/bin/fish.rs b/src/bin/fish.rs index d7680f2cb..85c4898e6 100644 --- a/src/bin/fish.rs +++ b/src/bin/fish.rs @@ -41,6 +41,7 @@ use fish::{ fprintf, function, future_feature_flags as features, history::{self, start_private_mode}, io::IoChain, + libc::{getrusage64, timeval64}, nix::{getpid, isatty}, panic::panic_handler, parse_constants::{ParseErrorList, ParseTreeFlags}, @@ -55,14 +56,12 @@ use fish::{ }, reader::{reader_init, reader_read, term_copy_modes}, signal::{signal_clear_cancel, signal_unblock_all}, - threads::{self}, - topic_monitor, + threads, topic_monitor, wchar::prelude::*, wutil::waccess, }; use std::ffi::{CString, OsStr, OsString}; use std::fs::File; -use std::mem::MaybeUninit; use std::os::unix::prelude::*; use std::path::{Path, PathBuf}; use std::rc::Rc; @@ -104,23 +103,20 @@ struct FishCmdOpts { } /// Return a timeval converted to milliseconds. -#[allow(clippy::unnecessary_cast)] -fn tv_to_msec(tv: &libc::timeval) -> i64 { +fn tv_to_msec(tv: &timeval64) -> i64 { // milliseconds per second - let mut msec = tv.tv_sec as i64 * 1000; + let mut msec = tv.tv_sec * 1000; // microseconds per millisecond - msec += tv.tv_usec as i64 / 1000; + msec += tv.tv_usec / 1000; msec } fn print_rusage_self() { - let mut rs = MaybeUninit::uninit(); - if unsafe { libc::getrusage(libc::RUSAGE_SELF, rs.as_mut_ptr()) } != 0 { + let Some(rs) = getrusage64(libc::RUSAGE_SELF) else { let s = CString::new("getrusage").unwrap(); unsafe { libc::perror(s.as_ptr()) } return; - } - let rs: libc::rusage = unsafe { rs.assume_init() }; + }; let rss_kb = if cfg!(target_os = "macos") { // mac use bytes. rs.ru_maxrss / 1024 diff --git a/src/libc.c b/src/libc.c index b0188d622..c162f0fd2 100644 --- a/src/libc.c +++ b/src/libc.c @@ -249,3 +249,22 @@ int C_select64(int nfds, fd_set* readfds, fd_set* writefds, fd_set* errorfds, } return result; } + +struct rusage64 { + struct timeval64 ru_utime; + struct timeval64 ru_stime; + int64_t ru_maxrss; + int64_t ru_nsignals; +}; + +int C_getrusage64(int resource, struct rusage64* usage) { + struct rusage tmp; + int result = getrusage(resource, &tmp); + usage->ru_utime.tv_sec = tmp.ru_utime.tv_sec; + usage->ru_utime.tv_usec = tmp.ru_utime.tv_usec; + usage->ru_stime.tv_sec = tmp.ru_stime.tv_sec; + usage->ru_stime.tv_usec = tmp.ru_stime.tv_usec; + usage->ru_maxrss = tmp.ru_maxrss; + usage->ru_nsignals = tmp.ru_nsignals; + return result; +} diff --git a/src/libc.rs b/src/libc.rs index b8a33196d..3d9ad3caf 100644 --- a/src/libc.rs +++ b/src/libc.rs @@ -126,7 +126,7 @@ extern "C" { #[repr(C)] #[derive(Clone, Copy)] -pub(crate) struct timeval64 { +pub struct timeval64 { pub tv_sec: i64, pub tv_usec: i64, } @@ -149,3 +149,27 @@ extern "C" { timeout: *mut timeval64, ) -> c_int; } + +#[repr(C)] +pub struct rusage64 { + pub ru_utime: timeval64, + pub ru_stime: timeval64, + pub ru_maxrss: u64, + pub ru_nsignals: u64, +} + +pub fn getrusage64(resource: c_int) -> Option { + let mut rusage = std::mem::MaybeUninit::uninit(); + let result = unsafe { C_getrusage64(resource, rusage.as_mut_ptr()) }; + // getrusage(2) says the syscall can only fail if the dest address is invalid (EFAULT) or if the + // requested resource type is invalid. Since we're in control of both, we can assume it won't + // fail. In case it does anyway (e.g. OS where the syscall isn't implemented), we can just + // return an empty value. + match result { + 0 => unsafe { Some(rusage.assume_init()) }, + _ => None, + } +} +extern "C" { + fn C_getrusage64(resource: c_int, usage: *mut rusage64) -> c_int; +} diff --git a/src/nix.rs b/src/nix.rs index ba0989748..2259f92bb 100644 --- a/src/nix.rs +++ b/src/nix.rs @@ -2,9 +2,10 @@ use std::time::Duration; -#[allow(clippy::unnecessary_cast)] -pub const fn timeval_to_duration(val: &libc::timeval) -> Duration { - let micros = val.tv_sec as i64 * (1E6 as i64) + val.tv_usec as i64; +use crate::libc::timeval64; + +pub(crate) const fn timeval_to_duration(val: &timeval64) -> Duration { + let micros = val.tv_sec * (1E6 as i64) + val.tv_usec; Duration::from_micros(micros as u64) } @@ -13,7 +14,7 @@ pub trait TimevalExt { fn as_duration(&self) -> Duration; } -impl TimevalExt for libc::timeval { +impl TimevalExt for timeval64 { fn as_micros(&self) -> i64 { timeval_to_duration(self).as_micros() as i64 } diff --git a/src/timer.rs b/src/timer.rs index ae09a84e7..ba6cf225a 100644 --- a/src/timer.rs +++ b/src/timer.rs @@ -17,6 +17,8 @@ use std::io::Write; use std::time::{Duration, Instant}; +use crate::libc::{getrusage64, rusage64}; + enum Unit { Minutes, Seconds, @@ -26,8 +28,8 @@ enum Unit { struct TimerSnapshot { wall_time: Instant, - cpu_fish: libc::rusage, - cpu_children: libc::rusage, + cpu_fish: rusage64, + cpu_children: rusage64, } /// Create a `TimerSnapshot` and return a `PrintElapsedOnDrop` object that will print upon @@ -45,24 +47,12 @@ enum RUsage { RChildren, } -/// A safe wrapper around `libc::getrusage()` -fn getrusage(resource: RUsage) -> libc::rusage { - let mut rusage = std::mem::MaybeUninit::uninit(); - let result = unsafe { - match resource { - RUsage::RSelf => libc::getrusage(libc::RUSAGE_SELF, rusage.as_mut_ptr()), - RUsage::RChildren => libc::getrusage(libc::RUSAGE_CHILDREN, rusage.as_mut_ptr()), - } - }; - - // getrusage(2) says the syscall can only fail if the dest address is invalid (EFAULT) or if the - // requested resource type is invalid. Since we're in control of both, we can assume it won't - // fail. In case it does anyway (e.g. OS where the syscall isn't implemented), we can just - // return an empty value. - match result { - 0 => unsafe { rusage.assume_init() }, - _ => unsafe { std::mem::zeroed() }, - } +fn getrusage(resource: RUsage) -> rusage64 { + getrusage64(match resource { + RUsage::RSelf => libc::RUSAGE_SELF, + RUsage::RChildren => libc::RUSAGE_CHILDREN, + }) + .unwrap_or(unsafe { std::mem::zeroed() }) } impl TimerSnapshot {