From 83599bd1393635078f2edc546dc7413321d5ff29 Mon Sep 17 00:00:00 2001 From: favilances Date: Wed, 13 May 2026 19:02:51 +0300 Subject: [PATCH] renice: accept documented target options renice documents util-linux style target selectors such as -p, -g, and -u, but clap rejected those options before the command reached its implementation. Parse the documented priority and target options and add focused coverage for option forms so the parser does not regress. Signed-off-by: favilances --- src/uu/renice/src/renice.rs | 177 ++++++++++++++++++++++++++++++----- tests/by-util/test_renice.rs | 30 ++++++ 2 files changed, 184 insertions(+), 23 deletions(-) diff --git a/src/uu/renice/src/renice.rs b/src/uu/renice/src/renice.rs index 2f391639..f923fb43 100644 --- a/src/uu/renice/src/renice.rs +++ b/src/uu/renice/src/renice.rs @@ -3,10 +3,9 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -use clap::{crate_version, Arg, Command}; +use clap::{crate_version, Arg, ArgAction, ArgGroup, Command}; #[cfg(not(windows))] -use libc::PRIO_PROCESS; -use std::env; +use libc::{PRIO_PGRP, PRIO_PROCESS, PRIO_USER}; #[cfg(not(windows))] use std::io::Error; use std::process; @@ -15,51 +14,183 @@ use uucore::{error::UResult, format_usage, help_about, help_usage}; const ABOUT: &str = help_about!("renice.md"); const USAGE: &str = help_usage!("renice.md"); +#[derive(Clone, Copy)] +enum TargetKind { + Process, + ProcessGroup, + User, +} + +impl TargetKind { + #[cfg(not(windows))] + fn which(self) -> u32 { + match self { + Self::Process => PRIO_PROCESS, + Self::ProcessGroup => PRIO_PGRP, + Self::User => PRIO_USER, + } + } + + fn label(self) -> &'static str { + match self { + Self::Process => "process ID", + Self::ProcessGroup => "process group ID", + Self::User => "user ID", + } + } +} + #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().try_get_matches_from(args)?; + let arguments = matches + .get_many::("arguments") + .unwrap() + .collect::>(); + + let priority_option = matches.get_one::("priority_option"); + let relative_option = matches.get_one::("relative"); + let (nice_value_str, identifiers, relative) = + if let Some(nice_value_str) = priority_option.or(relative_option) { + ( + nice_value_str, + arguments.as_slice(), + relative_option.is_some(), + ) + } else { + let Some((nice_value_str, identifiers)) = arguments.split_first() else { + eprintln!("Invalid nice value"); + process::exit(1); + }; + (*nice_value_str, identifiers, false) + }; + + if identifiers.is_empty() { + eprintln!("Invalid identifier"); + process::exit(1); + } - let nice_value_str = matches.get_one::("nice_value").unwrap(); // Retrieve as String let nice_value = nice_value_str.parse::().unwrap_or_else(|_| { eprintln!("Invalid nice value"); process::exit(1); }); - let pid_str = matches.get_one::("pid").unwrap(); // Retrieve as String - let pid = pid_str.parse::().unwrap_or_else(|_| { - eprintln!("Invalid PID"); - process::exit(1); - }); + let target_kind = if matches.get_flag("pgrp") { + TargetKind::ProcessGroup + } else if matches.get_flag("user") { + TargetKind::User + } else { + TargetKind::Process + }; - // TODO: implement functionality on windows - #[cfg(not(windows))] - if unsafe { libc::setpriority(PRIO_PROCESS, pid.try_into().unwrap(), nice_value) } == -1 { - eprintln!("Failed to set nice value: {}", Error::last_os_error()); - process::exit(1); + for identifier in identifiers { + let id = identifier.parse().unwrap_or_else(|_| { + eprintln!("Invalid {}", target_kind.label()); + process::exit(1); + }); + + set_nice_value(target_kind, id, nice_value, relative); } - println!("Nice value of process {pid} set to {nice_value}"); Ok(()) } +#[cfg(not(windows))] +fn set_nice_value(target_kind: TargetKind, id: u32, nice_value: i32, relative: bool) { + let which = target_kind.which(); + let nice_value = if relative { + let current = unsafe { libc::getpriority(which, id) }; + if current == -1 { + eprintln!( + "Failed to get nice value for {} {id}: {}", + target_kind.label(), + Error::last_os_error() + ); + process::exit(1); + } + current + nice_value + } else { + nice_value + }; + + if unsafe { libc::setpriority(which, id, nice_value) } == -1 { + eprintln!( + "Failed to set nice value for {} {id}: {}", + target_kind.label(), + Error::last_os_error() + ); + process::exit(1); + } + + println!( + "Nice value of {} {id} set to {nice_value}", + target_kind.label() + ); +} + +// TODO: implement functionality on windows +#[cfg(windows)] +fn set_nice_value(_target_kind: TargetKind, id: u32, nice_value: i32, _relative: bool) { + println!("Nice value of process ID {id} set to {nice_value}"); +} + pub fn uu_app() -> Command { Command::new(uucore::util_name()) .version(crate_version!()) .about(ABOUT) .override_usage(format_usage(USAGE)) .infer_long_args(true) + .group( + ArgGroup::new("priority") + .args(["priority_option", "relative"]) + .multiple(false), + ) + .group( + ArgGroup::new("target") + .args(["pid", "pgrp", "user"]) + .multiple(false), + ) .arg( - Arg::new("nice_value") + Arg::new("priority_option") + .short('n') + .long("priority") .value_name("NICE_VALUE") - .help("The new nice value for the process") - .required(true) - .index(1), + .allow_negative_numbers(true) + .help("The new nice value for the process"), + ) + .arg( + Arg::new("relative") + .long("relative") + .value_name("NICE_VALUE") + .allow_negative_numbers(true) + .help("Adjust the nice value relative to the current value"), ) .arg( Arg::new("pid") - .value_name("PID") - .help("The PID of the process") - .required(true) - .index(2), + .short('p') + .long("pid") + .action(ArgAction::SetTrue) + .help("Interpret identifiers as process IDs"), + ) + .arg( + Arg::new("pgrp") + .short('g') + .long("pgrp") + .action(ArgAction::SetTrue) + .help("Interpret identifiers as process group IDs"), + ) + .arg( + Arg::new("user") + .short('u') + .long("user") + .action(ArgAction::SetTrue) + .help("Interpret identifiers as user IDs"), + ) + .arg( + Arg::new("arguments") + .value_name("NICE_VALUE IDENTIFIER...") + .allow_negative_numbers(true) + .num_args(1..) + .required(true), ) } diff --git a/tests/by-util/test_renice.rs b/tests/by-util/test_renice.rs index cb9d3b1f..2222db5a 100644 --- a/tests/by-util/test_renice.rs +++ b/tests/by-util/test_renice.rs @@ -10,3 +10,33 @@ use uutests::new_ucmd; fn test_invalid_arg() { new_ucmd!().arg("--definitely-invalid").fails().code_is(1); } + +#[test] +fn test_pid_option_after_priority() { + new_ucmd!() + .args(&["19", "-p", "not-a-pid"]) + .fails() + .code_is(1) + .no_stdout() + .stderr_is("Invalid process ID\n"); +} + +#[test] +fn test_priority_option_before_pid() { + new_ucmd!() + .args(&["-n", "19", "-p", "not-a-pid"]) + .fails() + .code_is(1) + .no_stdout() + .stderr_is("Invalid process ID\n"); +} + +#[test] +fn test_pgrp_option_after_priority() { + new_ucmd!() + .args(&["19", "-g", "not-a-pgrp"]) + .fails() + .code_is(1) + .no_stdout() + .stderr_is("Invalid process group ID\n"); +}