From 47e2ac59540c52da003f36647fbecd53539cf686 Mon Sep 17 00:00:00 2001 From: mmc Date: Fri, 21 Aug 2026 11:00:53 -0500 Subject: [PATCH 1/3] ionice: implement the ionice utility ionice reads and sets the I/O scheduling class and priority of a process through ioprio_get and ioprio_set. The option surface follows ionice(1): -c, -n, -p, -P, -u and -t, with trailing operands being further ids when an id option was given and the command to run otherwise. Behavior throughout is what util-linux 2.42.2 was observed doing; no C source was read. The parts a reader should not have to reverse engineer: - The idle class carries priority level 7 and none carries 0, whatever -n asks for. Every idle priority prints as the bare word "idle" regardless of the level packed with it, so stdout cannot show this - only the syscall arguments can. None is forced by the kernel, which refuses a none class carrying a nonzero level; idle's 7 is the reference's convention. - A lone id of 0 on the read path means the calling process rather than the group or user numbered 0, and only when there are no further ids and no priority is being set. Writing never falls back: -c 3 -u 0 really does target uid 0. - Options are diagnosed left to right, reconstructed from clap's value indices, so the leftmost bad one is reported. A second id option is rejected before its value is looked at. - -c outside 0-3 and -n outside 0-7 are rejected with exit 1. ionice(1) documents both ranges and util-linux 2.42.2 enforces neither: it hands the value to the kernel, which reinterprets it wherever the reinterpretation is legal, so -c 99 lands on idle and -n 8192 changes the class outright, both exiting 0, and an accepted -n 8 really means level 0 because the kernel reads the level modulo eight. Where the wrap lands on something the kernel will not take, as with -c 4 and -n -1, the set fails EINVAL and the reference exits 1 - which is what we exit as well. This is a deliberate divergence in which argv succeed, put to the maintainers as uutils/util-linux#624 and answered with "you can enforce and document it". The disclosure belongs in the pull request body, so ionice.md and --help are left alone on purpose. One rider comes with it: the "unknown prio class N" warning no longer exists, because no class can be unknown any more, so -t has one fewer diagnostic to silence. - A value too wide for an i32 keeps its ERANGE wording, which is a failure the reference produces as well, and it is reported ahead of the documented range. - ioprio.rs owns the ioprio word - its bit layout, the class numbers and names, how it renders, and the two syscalls that carry it, which are the crate's only unsafe. ionice.rs owns what this tool decides to put in that word. The class stays an i32 rather than becoming an enum because the kernel treats it as an open value. Known divergences, each structural to clap rather than a defect in the port: the --help and --version layout, clap's diagnostics where getopt has its own, and clap stripping the = from an attached short-option value, which is tree-wide and which merged utilities already carry. --- Cargo.lock | 11 + Cargo.toml | 2 + src/uu/ionice/Cargo.toml | 18 ++ src/uu/ionice/ionice.md | 10 + src/uu/ionice/src/errors.rs | 107 ++++++++++ src/uu/ionice/src/ionice.rs | 405 ++++++++++++++++++++++++++++++++++++ src/uu/ionice/src/ioprio.rs | 117 +++++++++++ src/uu/ionice/src/main.rs | 1 + src/uu/ionice/src/parse.rs | 81 ++++++++ 9 files changed, 752 insertions(+) create mode 100644 src/uu/ionice/Cargo.toml create mode 100644 src/uu/ionice/ionice.md create mode 100644 src/uu/ionice/src/errors.rs create mode 100644 src/uu/ionice/src/ionice.rs create mode 100644 src/uu/ionice/src/ioprio.rs create mode 100644 src/uu/ionice/src/main.rs create mode 100644 src/uu/ionice/src/parse.rs diff --git a/Cargo.lock b/Cargo.lock index 8ecdb0bc..c33580e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1563,6 +1563,7 @@ dependencies = [ "uu_dmesg", "uu_fsfreeze", "uu_hexdump", + "uu_ionice", "uu_last", "uu_lscpu", "uu_lsipc", @@ -1658,6 +1659,16 @@ dependencies = [ "uucore 0.2.2", ] +[[package]] +name = "uu_ionice" +version = "0.0.1" +dependencies = [ + "clap", + "libc", + "thiserror", + "uucore 0.2.2", +] + [[package]] name = "uu_last" version = "0.0.1" diff --git a/Cargo.toml b/Cargo.toml index 40a96aa0..8d3d04a7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,7 @@ feat_common_core = [ "dmesg", "fsfreeze", "hexdump", + "ionice", "last", "lscpu", "lsipc", @@ -106,6 +107,7 @@ ctrlaltdel = { optional = true, version = "0.0.1", package = "uu_ctrlaltdel", pa dmesg = { optional = true, version = "0.0.1", package = "uu_dmesg", path = "src/uu/dmesg" } fsfreeze = { optional = true, version = "0.0.1", package = "uu_fsfreeze", path = "src/uu/fsfreeze" } hexdump = { optional = true, version = "0.0.1", package = "uu_hexdump", path = "src/uu/hexdump" } +ionice = { optional = true, version = "0.0.1", package = "uu_ionice", path = "src/uu/ionice" } last = { optional = true, version = "0.0.1", package = "uu_last", path = "src/uu/last" } lscpu = { optional = true, version = "0.0.1", package = "uu_lscpu", path = "src/uu/lscpu" } lsipc = { optional = true, version = "0.0.1", package = "uu_lsipc", path = "src/uu/lsipc" } diff --git a/src/uu/ionice/Cargo.toml b/src/uu/ionice/Cargo.toml new file mode 100644 index 00000000..fee49435 --- /dev/null +++ b/src/uu/ionice/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "uu_ionice" +version = "0.0.1" +edition = "2021" +description = "ionice ~ (uutils) Show or change the I/O-scheduling class and priority of a process" + +[lib] +path = "src/ionice.rs" + +[[bin]] +name = "ionice" +path = "src/main.rs" + +[dependencies] +clap = { workspace = true } +libc = { workspace = true } +thiserror = { workspace = true } +uucore = { workspace = true } diff --git a/src/uu/ionice/ionice.md b/src/uu/ionice/ionice.md new file mode 100644 index 00000000..2789ba86 --- /dev/null +++ b/src/uu/ionice/ionice.md @@ -0,0 +1,10 @@ +# ionice + +``` +ionice [options] -p ... +ionice [options] -P ... +ionice [options] -u ... +ionice [options] +``` + +Show or change the I/O-scheduling class and priority of a process diff --git a/src/uu/ionice/src/errors.rs b/src/uu/ionice/src/errors.rs new file mode 100644 index 00000000..05c61d6d --- /dev/null +++ b/src/uu/ionice/src/errors.rs @@ -0,0 +1,107 @@ +// This file is part of the uutils util-linux package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. +// spell-checker:ignore (words) ERANGE ioprio pgid strerror + +use std::fmt; +use std::io; + +use uucore::error::{strip_errno, UError}; + +/// Which option a numeric argument came from. Supplies the middle of the +/// "invalid ... argument" message. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum NumericArg { + Class, + ClassData, + Pid, + Pgid, + Uid, +} + +impl fmt::Display for NumericArg { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + Self::Class => "class", + Self::ClassData => "class data", + Self::Pid => "PID", + Self::Pgid => "PGID", + Self::Uid => "UID", + }) + } +} + +/// Trailing detail on a numeric argument error: nothing when the text was +/// merely malformed, strerror(ERANGE) when it did not fit in an i32, and the +/// bounds themselves when it fell outside a range ionice(1) documents. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum NumericDetail { + Malformed, + Overflow, + OutOfRange { low: i32, high: i32 }, +} + +impl fmt::Display for NumericDetail { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Malformed => Ok(()), + Self::Overflow => { + let range = io::Error::from_raw_os_error(libc::ERANGE); + write!(f, ": {}", strip_errno(&range)) + } + Self::OutOfRange { low, high } => write!(f, ": must be {low}-{high}"), + } + } +} + +#[derive(Debug, thiserror::Error)] +pub(crate) enum IoniceError { + #[error("invalid {arg} argument: '{value}'{detail}")] + InvalidNumber { + arg: NumericArg, + value: String, + detail: NumericDetail, + }, + + #[error("unknown scheduling class: '{0}'")] + UnknownClass(String), + + #[error("can handle only one of pid, pgid or uid at once")] + ConflictingIdKinds, + + // The hint is built here rather than left to UError::usage(), which spells + // out execution_phrase() - the whole multicall binary path - instead of the + // utility name. + #[error( + "bad usage\nTry '{} --help' for more information.", + uucore::util_name() + )] + BadUsage, + + #[error("ioprio_get failed: {}", strip_errno(.0))] + GetFailed(io::Error), + + #[error("ioprio_set failed: {}", strip_errno(.0))] + SetFailed(io::Error), +} + +impl IoniceError { + pub(crate) fn invalid_number(arg: NumericArg, value: &str, detail: NumericDetail) -> Self { + Self::InvalidNumber { + arg, + value: value.to_owned(), + detail, + } + } +} + +impl UError for IoniceError { + fn code(&self) -> i32 { + 1 + } + + fn usage(&self) -> bool { + false + } +} diff --git a/src/uu/ionice/src/ionice.rs b/src/uu/ionice/src/ionice.rs new file mode 100644 index 00000000..3d6256a7 --- /dev/null +++ b/src/uu/ionice/src/ionice.rs @@ -0,0 +1,405 @@ +// This file is part of the uutils util-linux package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. +// spell-checker:ignore (words) classdata ioprio pgid pgrp + +use clap::builder::ValueParser; +use clap::{crate_version, Arg, ArgAction, Command}; +use uucore::{error::UResult, format_usage, help_about, help_usage}; + +#[cfg(target_os = "linux")] +mod errors; +#[cfg(target_os = "linux")] +mod ioprio; +#[cfg(target_os = "linux")] +mod parse; + +const ABOUT: &str = help_about!("ionice.md"); +const USAGE: &str = help_usage!("ionice.md"); + +mod options { + pub const CLASS: &str = "class"; + pub const CLASSDATA: &str = "classdata"; + pub const PID: &str = "pid"; + pub const PGID: &str = "pgid"; + pub const UID: &str = "uid"; + pub const IGNORE: &str = "ignore"; + pub const ARGS: &str = "args"; +} + +#[cfg(target_os = "linux")] +mod linux { + use std::ffi::OsString; + use std::io; + use std::os::unix::process::CommandExt; + use std::process; + + use clap::ArgMatches; + use uucore::error::UResult; + + use crate::errors::{IoniceError, NumericArg}; + use crate::ioprio::{self, IoPrio, Who}; + use crate::options; + use crate::parse::{self, ClassError}; + + /// Which of -p, -P and -u was given. Only one of them may appear. + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + enum IdKind { + Pid, + Pgid, + Uid, + } + + impl IdKind { + fn who(self) -> Who { + match self { + Self::Pid => Who::Process, + Self::Pgid => Who::Pgrp, + Self::Uid => Who::User, + } + } + + fn numeric_arg(self) -> NumericArg { + match self { + Self::Pid => NumericArg::Pid, + Self::Pgid => NumericArg::Pgid, + Self::Uid => NumericArg::Uid, + } + } + } + + /// A value-taking option, tagged for the ordered pass below. + #[derive(Debug, Clone, Copy)] + enum Slot { + Class, + ClassData, + Id(IdKind), + } + + /// What the command line asked for once the options have been resolved. + #[derive(Debug, Default)] + struct Request { + class: Option, + data: Option, + id_kind: Option, + first_id: i32, + ignore: bool, + } + + /// Rebuild the left-to-right order in which the value-taking options + /// appeared. clap collects every value before returning, but its value + /// indices are monotone in argv order, so sorting by them recovers the + /// sequence needed to reject the first bad option rather than an arbitrary + /// one. + fn ordered_options(matches: &ArgMatches) -> Vec<(usize, Slot, String)> { + let slots = [ + (options::CLASS, Slot::Class), + (options::CLASSDATA, Slot::ClassData), + (options::PID, Slot::Id(IdKind::Pid)), + (options::PGID, Slot::Id(IdKind::Pgid)), + (options::UID, Slot::Id(IdKind::Uid)), + ]; + + let mut events = Vec::new(); + + for (id, slot) in slots { + let (Some(values), Some(indices)) = + (matches.get_many::(id), matches.indices_of(id)) + else { + continue; + }; + + events.extend( + indices + .zip(values) + .map(|(index, value)| (index, slot, value.to_string_lossy().into_owned())), + ); + } + + events.sort_by_key(|(index, _, _)| *index); + events + } + + /// Apply the options in argv order, stopping at the first bad one. + fn resolve_options(matches: &ArgMatches) -> Result { + let mut request = Request { + ignore: matches.get_count(options::IGNORE) > 0, + ..Request::default() + }; + + for (_, slot, value) in ordered_options(matches) { + match slot { + Slot::Class => { + let class = parse::class(&value).map_err(|error| match error { + ClassError::UnknownName => IoniceError::UnknownClass(value.clone()), + ClassError::Number(detail) => { + IoniceError::invalid_number(NumericArg::Class, &value, detail) + } + })?; + request.class = Some(class); + } + Slot::ClassData => { + let data = parse::level(&value).map_err(|detail| { + IoniceError::invalid_number(NumericArg::ClassData, &value, detail) + })?; + request.data = Some(data); + } + Slot::Id(kind) => { + // A second id option is rejected before its value is even + // looked at, so `-P 1 -p bogus` reports the conflict. + if request.id_kind.is_some() { + return Err(IoniceError::ConflictingIdKinds); + } + request.id_kind = Some(kind); + request.first_id = parse_id(kind, &value)?; + } + } + } + + Ok(request) + } + + fn parse_id(kind: IdKind, value: &str) -> Result { + parse::blank_tolerant_i32(value) + .map_err(|detail| IoniceError::invalid_number(kind.numeric_arg(), value, detail)) + } + + /// What a set uses when -c or -n is absent. These are ionice's defaults, + /// not the kernel's; the kernel's own fallback is derived from CPU nice, + /// not from these numbers. + const DEFAULT_CLASS: i32 = ioprio::IOPRIO_CLASS_BE; + const DEFAULT_DATA: i32 = 4; + + /// The priority level a class carries when it has no level of its own; + /// a level given with -n is discarded. + /// + /// The two arms have different authorities behind them. None must carry + /// zero because that is the only value the kernel accepts for it. Idle + /// carrying 7 is the reference implementation's convention: the kernel + /// ignores idle's level entirely, so nothing forces that choice. + fn fixed_data(class: i32) -> Option { + match class { + ioprio::IOPRIO_CLASS_NONE => Some(0), + ioprio::IOPRIO_CLASS_IDLE => Some(7), + _ => None, + } + } + + /// Settle the class and priority level a set will use. The warning is + /// silenced by -t, but the clearing of a meaningless priority level is not. + fn resolve_priority(request: &Request) -> IoPrio { + let class = request.class.unwrap_or(DEFAULT_CLASS); + + if let Some(data) = fixed_data(class) { + // The level is replaced whether or not -n asked for one; only + // saying so is conditional. + if request.data.is_some() && !request.ignore { + uucore::show_error!( + "ignoring given class data for {} class", + ioprio::class_name(class) + ); + } + return IoPrio::encode(class, data); + } + + IoPrio::encode(class, request.data.unwrap_or(DEFAULT_DATA)) + } + + pub(crate) fn run(matches: &ArgMatches) -> UResult<()> { + let request = resolve_options(matches)?; + + let arguments: Vec = matches + .get_many::(options::ARGS) + .map(|values| values.cloned().collect()) + .unwrap_or_default(); + + // Trailing arguments are further ids whenever an id option was given, + // and the command to run otherwise. + let command = if request.id_kind.is_none() && !arguments.is_empty() { + Some(arguments.as_slice()) + } else { + None + }; + + // Running a command always sets a priority, defaulting to best-effort + // level 4. The warning belongs here rather than in the dispatch below, + // because `ionice -c 3 -n 5` reports the discarded level before the + // bad usage. + let priority = (request.class.is_some() || request.data.is_some() || command.is_some()) + .then(|| resolve_priority(&request)); + + match (request.id_kind, command, priority) { + // A get whose only id is 0 reports the calling process rather than + // the process group or user numbered 0. Setting has no such + // fallback: `-c 3 -u 0` really does target uid 0. + (Some(_), _, None) if request.first_id == 0 && arguments.is_empty() => { + report(Who::Process, 0) + } + (Some(kind), _, _) => act_on_ids(&request, kind, priority, &arguments), + (None, Some(command), Some(priority)) => { + apply(&request, Who::Process, 0, priority)?; + run_command(command) + } + (None, None, Some(_)) => Err(IoniceError::BadUsage.into()), + (None, _, None) => report(Who::Process, 0), + } + } + + /// Act on the id given to the option, then on every trailing argument, + /// parsing each one only when its turn comes so that a bad id later in the + /// list does not hide the results of the ids before it. + fn act_on_ids( + request: &Request, + kind: IdKind, + priority: Option, + arguments: &[OsString], + ) -> UResult<()> { + let who = kind.who(); + act_on_id(request, who, request.first_id, priority)?; + + for argument in arguments { + let id = parse_id(kind, &argument.to_string_lossy())?; + act_on_id(request, who, id, priority)?; + } + + Ok(()) + } + + fn act_on_id(request: &Request, who: Who, id: i32, priority: Option) -> UResult<()> { + match priority { + Some(priority) => apply(request, who, id, priority), + None => report(who, id), + } + } + + fn report(who: Who, id: i32) -> UResult<()> { + let priority = ioprio::get(who, id).map_err(IoniceError::GetFailed)?; + println!("{priority}"); + Ok(()) + } + + /// Set a priority, honoring -t: a failure to set is then silent and leaves + /// the exit status alone. + fn apply(request: &Request, who: Who, id: i32, priority: IoPrio) -> UResult<()> { + match ioprio::set(who, id, priority) { + Ok(()) => Ok(()), + Err(_) if request.ignore => Ok(()), + Err(error) => Err(IoniceError::SetFailed(error).into()), + } + } + + /// Replace this process with the command. ionice does not fork, so the + /// command inherits the priority just set and its exit status is naturally + /// this process's own. + fn run_command(command: &[OsString]) -> UResult<()> { + let Some((program, arguments)) = command.split_first() else { + return Ok(()); + }; + + // exec() returns only when it failed. + let error = process::Command::new(program).args(arguments).exec(); + + uucore::show_error!( + "failed to execute {}: {}", + program.to_string_lossy(), + uucore::error::strip_errno(&error) + ); + uucore::error::set_exit_code(if error.kind() == io::ErrorKind::NotFound { + 127 + } else { + 126 + }); + + Ok(()) + } +} + +#[cfg(target_os = "linux")] +#[uucore::main] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let matches: clap::ArgMatches = uu_app().try_get_matches_from(args)?; + + linux::run(&matches) +} + +#[cfg(not(target_os = "linux"))] +#[uucore::main] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let _matches: clap::ArgMatches = uu_app().try_get_matches_from(args)?; + + Err(uucore::error::USimpleError::new( + 1, + "`ionice` is available only on Linux.", + )) +} + +/// A value-taking option. Hyphen-leading values are accepted so that the +/// argument following the flag is taken verbatim, which is what makes +/// `ionice -p -c 3` report an invalid PID of "-c" instead of parsing -c. +fn value_option( + id: &'static str, + short: char, + long: &'static str, + value_name: &'static str, +) -> Arg { + Arg::new(id) + .short(short) + .long(long) + .value_name(value_name) + .action(ArgAction::Append) + .num_args(1) + .allow_hyphen_values(true) + .value_parser(ValueParser::os_string()) +} + +pub fn uu_app() -> Command { + Command::new(uucore::util_name()) + .version(crate_version!()) + .about(ABOUT) + .override_usage(format_usage(USAGE)) + .infer_long_args(true) + .arg(value_option(options::CLASS, 'c', "class", "class").help( + "name or number of the scheduling class: \ + none (0), realtime (1), best-effort (2) or idle (3)", + )) + .arg( + value_option(options::CLASSDATA, 'n', "classdata", "num").help( + "priority (0..7) in the specified scheduling class, \ + only for the realtime and best-effort classes", + ), + ) + .arg( + value_option(options::PID, 'p', "pid", "pid") + .help("act on these already running processes"), + ) + .arg( + value_option(options::PGID, 'P', "pgid", "pgrp") + .help("act on already running processes in these groups"), + ) + // Declared between -P and -u so that --help lists the options in the + // same order the reference does; clap orders by declaration. + .arg( + Arg::new(options::IGNORE) + .short('t') + .long("ignore") + // Count rather than SetTrue: clap rejects a repeated SetTrue + // flag, and `ionice -t -t` is accepted. + .action(ArgAction::Count) + .help("ignore failures"), + ) + .arg( + value_option(options::UID, 'u', "uid", "uid") + .help("act on already running processes owned by these users"), + ) + .arg( + Arg::new(options::ARGS) + .value_name("command") + .help("further pid, pgrp or uid arguments, or the command to run") + .index(1) + .action(ArgAction::Set) + .trailing_var_arg(true) + .value_parser(ValueParser::os_string()) + .num_args(1..), + ) +} diff --git a/src/uu/ionice/src/ioprio.rs b/src/uu/ionice/src/ioprio.rs new file mode 100644 index 00000000..792f1916 --- /dev/null +++ b/src/uu/ionice/src/ioprio.rs @@ -0,0 +1,117 @@ +// This file is part of the uutils util-linux package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. +// spell-checker:ignore (words) ioprio pgrp + +use std::ffi::{c_int, c_long}; +use std::fmt; +use std::io; + +pub(crate) const IOPRIO_CLASS_NONE: i32 = 0; +pub(crate) const IOPRIO_CLASS_RT: i32 = 1; +pub(crate) const IOPRIO_CLASS_BE: i32 = 2; +pub(crate) const IOPRIO_CLASS_IDLE: i32 = 3; + +/// The scheduling class names, accepted by -c and printed back; the numbers +/// are the kernel's. +pub(crate) const CLASSES: [(&str, i32); 4] = [ + ("none", IOPRIO_CLASS_NONE), + ("realtime", IOPRIO_CLASS_RT), + ("best-effort", IOPRIO_CLASS_BE), + ("idle", IOPRIO_CLASS_IDLE), +]; + +/// The kernel packs a scheduling class above this many bits of priority data. +const IOPRIO_CLASS_SHIFT: u32 = 13; +const IOPRIO_PRIO_MASK: i32 = (1 << IOPRIO_CLASS_SHIFT) - 1; +const IOPRIO_CLASS_MASK: i32 = 0x7; + +/// Whom an ioprio call applies to. libc exposes no IOPRIO_WHO_* constants. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[repr(i32)] +pub(crate) enum Who { + Process = 1, + Pgrp = 2, + User = 3, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct IoPrio(i32); + +impl IoPrio { + /// Pack a class and a priority level. Neither is masked, and neither + /// needs to be: the only classes and levels that reach here are the ones + /// ionice(1) documents, which occupy their fields exactly. + pub(crate) fn encode(class: i32, data: i32) -> Self { + Self((class << IOPRIO_CLASS_SHIFT) | data) + } + + pub(crate) fn class(self) -> i32 { + (self.0 >> IOPRIO_CLASS_SHIFT) & IOPRIO_CLASS_MASK + } + + pub(crate) fn data(self) -> i32 { + self.0 & IOPRIO_PRIO_MASK + } +} + +impl fmt::Display for IoPrio { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let class = self.class(); + + if class == IOPRIO_CLASS_IDLE { + // The idle class carries no priority level. + f.write_str(class_name(class)) + } else { + write!(f, "{}: prio {}", class_name(class), self.data()) + } + } +} + +/// The fallback is unreachable - the kernel constrains a get to classes 0 +/// through 3, and the one warning that names a class is raised only for none +/// or idle - but it keeps this from having to panic. +pub(crate) fn class_name(class: i32) -> &'static str { + CLASSES + .iter() + .find(|&&(_, value)| value == class) + .map_or("unknown", |&(name, _)| name) +} + +pub(crate) fn get(who: Who, who_id: i32) -> io::Result { + // SAFETY: ioprio_get takes two integers by value and touches no memory. + // c_long::from avoids a cast, because the syscall number is c_int on a few + // targets and c_long on the rest. + let result = unsafe { + libc::syscall( + c_long::from(libc::SYS_ioprio_get), + c_long::from(who as i32), + c_long::from(who_id), + ) + }; + + if result < 0 { + Err(io::Error::last_os_error()) + } else { + Ok(IoPrio(result as c_int)) + } +} + +pub(crate) fn set(who: Who, who_id: i32, priority: IoPrio) -> io::Result<()> { + // SAFETY: ioprio_set takes three integers by value and touches no memory. + let result = unsafe { + libc::syscall( + c_long::from(libc::SYS_ioprio_set), + c_long::from(who as i32), + c_long::from(who_id), + c_long::from(priority.0), + ) + }; + + if result < 0 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } +} diff --git a/src/uu/ionice/src/main.rs b/src/uu/ionice/src/main.rs new file mode 100644 index 00000000..146238c7 --- /dev/null +++ b/src/uu/ionice/src/main.rs @@ -0,0 +1 @@ +uucore::bin!(uu_ionice); diff --git a/src/uu/ionice/src/parse.rs b/src/uu/ionice/src/parse.rs new file mode 100644 index 00000000..63691129 --- /dev/null +++ b/src/uu/ionice/src/parse.rs @@ -0,0 +1,81 @@ +// This file is part of the uutils util-linux package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. +// spell-checker:ignore (words) ioprio isspace + +use std::num::IntErrorKind; +use std::ops::RangeInclusive; + +use crate::errors::NumericDetail; +use crate::ioprio::{CLASSES, IOPRIO_CLASS_IDLE, IOPRIO_CLASS_NONE}; + +/// The characters C's isspace() accepts, and which therefore may precede a +/// number. Note the vertical tab, which char::is_ascii_whitespace omits. +const BLANKS: [char; 6] = [' ', '\t', '\n', '\x0b', '\x0c', '\r']; + +/// The ranges ionice(1) documents for -c and -n. The reference enforces +/// neither, and the kernel only reinterprets: it takes the class from three +/// bits and reads the level modulo eight. So a wider value selects a class or +/// level the caller never named, unless the wrap lands on a class the kernel +/// refuses outright. Refusing it here is a deliberate divergence from the +/// reference; see uutils/util-linux#624. +const DOCUMENTED_CLASSES: RangeInclusive = IOPRIO_CLASS_NONE..=IOPRIO_CLASS_IDLE; +const DOCUMENTED_LEVELS: RangeInclusive = 0..=7; + +pub(crate) enum ClassError { + Number(NumericDetail), + UnknownName, +} + +/// A decimal i32 with no leading blanks: an optional sign, then digits, then +/// nothing else. Overflow is kept apart from the other parse failures so that +/// a well-formed but oversized value is reported as such rather than as +/// malformed. +pub(crate) fn strict_i32(text: &str) -> Result { + text.parse::().map_err(|error| match error.kind() { + IntErrorKind::PosOverflow | IntErrorKind::NegOverflow => NumericDetail::Overflow, + _ => NumericDetail::Malformed, + }) +} + +/// A decimal i32 that tolerates leading blanks, as -n, -p, -P and -u do. +pub(crate) fn blank_tolerant_i32(text: &str) -> Result { + strict_i32(text.trim_start_matches(|c: char| BLANKS.contains(&c))) +} + +/// Hold a value to a documented range, naming the bounds so that the message +/// carries its own remedy. Applied after the parse, so a value too wide for +/// an i32 is still reported as the overflow it is. +fn within(value: i32, range: &RangeInclusive) -> Result { + if range.contains(&value) { + Ok(value) + } else { + Err(NumericDetail::OutOfRange { + low: *range.start(), + high: *range.end(), + }) + } +} + +/// A -n value: a priority level, blanks tolerated, held to its range. +pub(crate) fn level(text: &str) -> Result { + within(blank_tolerant_i32(text)?, &DOCUMENTED_LEVELS) +} + +/// A -c value. A leading digit selects the numeric form, so " 3" and "-3" take +/// the name branch and are reported as unknown class names rather than as +/// malformed numbers. +pub(crate) fn class(text: &str) -> Result { + if text.starts_with(|c: char| c.is_ascii_digit()) { + strict_i32(text) + .and_then(|value| within(value, &DOCUMENTED_CLASSES)) + .map_err(ClassError::Number) + } else { + CLASSES + .iter() + .find(|(name, _)| name.eq_ignore_ascii_case(text)) + .map(|&(_, value)| value) + .ok_or(ClassError::UnknownName) + } +} From bee2a0e6ab8fe126b88a45af406e6d6bfcdaa4c5 Mon Sep 17 00:00:00 2001 From: mmc Date: Fri, 21 Aug 2026 11:01:07 -0500 Subject: [PATCH 2/3] ionice: add integration tests Fifty-five tests, plus one for the non-Linux arm: reading a priority by pid, pgid and uid, the lone-zero-id fallback, setting each class and level, the two classes that discard a given level, argument rejection on every numeric option, the command path with its 126 and 127 statuses, and the test_invalid_arg smoke test every other util carries. They defend the decisions a refactor would plausibly undo rather than restating what clap already guarantees. The idle level is read back with a direct syscall rather than through the tool's own encoder, because stdout prints the bare word "idle" whatever level is packed with it and an assertion sharing the encoder would agree with it wherever it was wrong. The lone-zero-id fallback, the left-to-right order of diagnosis, and the precedence of an i32 overflow over the documented range each have a test whose only job is to fail if someone tidies them away. Anything that needs a set to be refused is guarded on geteuid and skipped under root, which would let the set succeed - and in the pid 1 case would re-class init rather than earning the EPERM the test expects. CI is never root, so these run everywhere it matters. Diagnostics are asserted only as far as we write them; the errno text after our half belongs to libc. Two of them are about argument bytes rather than about ionice. The coreutils contributing guide asks that anything that could be an invalid-Unicode path stay OsStr, and here that is the command to exec: it is taken as an OsString and handed to process::Command untouched. Nothing proved it. A directory makes the discriminator cheap - exec refuses one with EACCES, so a 126 says the name reached the kernel intact where a mangled one would have named nothing and given 127. The other test holds the value options to the same rule from the other side: a -p value that is not valid Unicode has to reach our parser and be diagnosed as a bad PID, not be refused by clap first. Both were checked against a deliberately broken build before being kept: a String value parser on -p fails the second, a lossy Command::new fails the first, and neither test fails for the other's reason. --- tests/by-util/test_ionice.rs | 802 +++++++++++++++++++++++++++++++++++ tests/tests.rs | 4 + 2 files changed, 806 insertions(+) create mode 100644 tests/by-util/test_ionice.rs diff --git a/tests/by-util/test_ionice.rs b/tests/by-util/test_ionice.rs new file mode 100644 index 00000000..7caa2903 --- /dev/null +++ b/tests/by-util/test_ionice.rs @@ -0,0 +1,802 @@ +// This file is part of the uutils util-linux package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. +// spell-checker:ignore (words) EACCES classdata ioprio pgid strace + +use uutests::new_ucmd; + +#[test] +fn test_invalid_arg() { + new_ucmd!().arg("--definitely-invalid").fails().code_is(1); +} + +#[cfg(target_os = "linux")] +mod linux { + use regex::Regex; + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + use std::process::{Child, Command}; + use uutests::at_and_ucmd; + use uutests::new_ucmd; + use uutests::util::{get_tests_binary, UCommand}; + + /// Run our own ionice under the ionice being tested, so that the priority + /// just set can be read back from the process that inherited it. Without + /// this the assertions would depend on whatever priority the test runner + /// happens to be running at. + fn nested(options: &[&str]) -> UCommand { + let mut command = new_ucmd!(); + command.args(options).arg(get_tests_binary()).arg("ionice"); + command + } + + /// Any line a get can legitimately print. The priority of a process the + /// test did not create is not fixed, so tests that read one assert only its + /// shape. + fn class_line() -> Regex { + Regex::new(r"^(?:idle|(?:none|realtime|best-effort): prio \d+)\n$").unwrap() + } + + /// A process id that cannot exist: pid_max is bounded well below this. + const ABSENT_PID: &str = "2147483647"; + + // -- reading a priority -------------------------------------------------- + + #[test] + fn reports_the_calling_process_by_default() { + new_ucmd!() + .succeeds() + .no_stderr() + .stdout_matches(&class_line()); + } + + #[test] + fn ignore_alone_still_reads() { + new_ucmd!() + .arg("-t") + .succeeds() + .no_stderr() + .stdout_matches(&class_line()); + } + + /// The reference accepts a repeated -t, so the flag must stay countable + /// rather than become a boolean that rejects its second appearance. + #[test] + fn ignore_may_be_repeated() { + new_ucmd!() + .args(&["-t", "-t"]) + .succeeds() + .no_stderr() + .stdout_matches(&class_line()); + } + + #[test] + fn reads_by_pid() { + new_ucmd!() + .args(&["-p", "1"]) + .succeeds() + .no_stderr() + .stdout_matches(&class_line()); + } + + #[test] + fn reads_by_pgid() { + let group = unsafe { libc::getpgid(0) }; + new_ucmd!() + .args(&["-P", &group.to_string()]) + .succeeds() + .no_stderr() + .stdout_matches(&class_line()); + } + + #[test] + fn reads_by_uid() { + let user = unsafe { libc::getuid() }; + new_ucmd!() + .args(&["-u", &user.to_string()]) + .succeeds() + .no_stderr() + .stdout_matches(&class_line()); + } + + #[test] + fn reads_one_line_per_id() { + new_ucmd!() + .args(&["-p", "1", "1"]) + .succeeds() + .no_stderr() + .stdout_matches(&Regex::new(r"^(?:.+\n){2}$").unwrap()); + } + + /// An id of 0 on the read path means the calling process, not the group or + /// the user numbered 0. Reading it back under a known class is what makes + /// this observable. + #[test] + fn a_lone_zero_id_means_the_caller() { + for option in ["-u", "-P", "-p"] { + nested(&["-c", "3"]) + .args(&[option, "0"]) + .succeeds() + .no_stderr() + .stdout_is("idle\n"); + } + } + + #[test] + fn a_zero_id_with_more_ids_does_not_mean_the_caller() { + let user = unsafe { libc::getuid() }; + new_ucmd!() + .args(&["-u", "0", &user.to_string()]) + .succeeds() + .no_stderr() + .stdout_matches(&Regex::new(r"^(?:.+\n){2}$").unwrap()); + } + + #[test] + fn reading_an_absent_pid_fails() { + new_ucmd!() + .args(&["-p", ABSENT_PID]) + .fails() + .code_is(1) + .no_stdout() + .stderr_is("ionice: ioprio_get failed: No such process\n"); + } + + #[test] + fn ignore_does_not_silence_a_failed_read() { + new_ucmd!() + .args(&["-t", "-p", ABSENT_PID]) + .fails() + .code_is(1) + .stderr_is("ionice: ioprio_get failed: No such process\n"); + } + + /// Each id is parsed only when its turn comes, so the ids before a bad one + /// have already been reported. + #[test] + fn reading_stops_at_the_first_bad_id() { + new_ucmd!() + .args(&["-p", "1", "bogus"]) + .fails() + .code_is(1) + .stdout_matches(&class_line()) + .stderr_is("ionice: invalid PID argument: 'bogus'\n"); + } + + #[test] + fn reading_stops_at_the_first_absent_id() { + new_ucmd!() + .args(&["-p", ABSENT_PID, "1"]) + .fails() + .code_is(1) + .no_stdout() + .stderr_is("ionice: ioprio_get failed: No such process\n"); + } + + // -- setting a priority, read back through an exec'd child --------------- + + #[test] + fn defaults_to_best_effort_level_four() { + nested(&[]) + .succeeds() + .no_stderr() + .stdout_is("best-effort: prio 4\n"); + } + + #[test] + fn sets_class_by_number() { + nested(&["-c", "0"]) + .succeeds() + .no_stderr() + .stdout_is("none: prio 0\n"); + nested(&["-c", "2"]) + .succeeds() + .no_stderr() + .stdout_is("best-effort: prio 4\n"); + nested(&["-c", "3"]) + .succeeds() + .no_stderr() + .stdout_is("idle\n"); + } + + #[test] + fn sets_class_by_name_ignoring_case() { + for name in ["idle", "IDLE", "Idle"] { + nested(&["-c", name]) + .succeeds() + .no_stderr() + .stdout_is("idle\n"); + } + nested(&["-c", "best-effort"]) + .succeeds() + .no_stderr() + .stdout_is("best-effort: prio 4\n"); + } + + #[test] + fn class_names_must_be_spelled_in_full() { + new_ucmd!() + .args(&["-c", "be"]) + .fails() + .code_is(1) + .stderr_is("ionice: unknown scheduling class: 'be'\n"); + } + + #[test] + fn sets_level_within_the_class() { + nested(&["-c", "2", "-n", "0"]) + .succeeds() + .no_stderr() + .stdout_is("best-effort: prio 0\n"); + nested(&["-c", "2", "-n", "7"]) + .succeeds() + .no_stderr() + .stdout_is("best-effort: prio 7\n"); + } + + #[test] + fn a_level_without_a_class_means_best_effort() { + nested(&["-n", "6"]) + .succeeds() + .no_stderr() + .stdout_is("best-effort: prio 6\n"); + } + + /// Neither class takes a level, so one given with -n is dropped. + #[test] + fn idle_and_none_drop_a_given_level() { + nested(&["-c", "3", "-n", "5"]) + .succeeds() + .stdout_is("idle\n") + .stderr_is("ionice: ignoring given class data for idle class\n"); + nested(&["-c", "0", "-n", "5"]) + .succeeds() + .stdout_is("none: prio 0\n") + .stderr_is("ionice: ignoring given class data for none class\n"); + } + + #[test] + fn ignore_silences_the_dropped_level_warning() { + nested(&["-t", "-c", "3", "-n", "5"]) + .succeeds() + .no_stderr() + .stdout_is("idle\n"); + nested(&["-t", "-c", "0", "-n", "5"]) + .succeeds() + .no_stderr() + .stdout_is("none: prio 0\n"); + } + + #[test] + fn the_last_class_wins() { + nested(&["-c", "2", "-n", "5", "-c", "3"]) + .succeeds() + .stdout_is("idle\n") + .stderr_is("ionice: ignoring given class data for idle class\n"); + nested(&["-c", "3", "-n", "5", "-c", "2"]) + .succeeds() + .no_stderr() + .stdout_is("best-effort: prio 5\n"); + } + + /// ionice(1) documents -n as a level of 0 to 7 and the reference checks + /// nothing: the value goes into the ioprio word as it stands. So `-n 8` + /// stays best-effort but means level 0, the kernel reading the level + /// modulo eight; `-n 8192` carries into the class field and becomes idle; + /// and `-n -1` makes a word the kernel refuses. This port rejects all + /// three before any syscall - see uutils/util-linux#624. + #[test] + fn the_level_is_range_checked() { + for level in ["8", "8192", "-1"] { + new_ucmd!() + .args(&["-n", level, "true"]) + .fails() + .code_is(1) + .no_stdout() + .stderr_is(format!( + "ionice: invalid class data argument: '{level}': must be 0-7\n" + )); + } + } + + /// The level is judged as it is parsed, before the class has any say, so + /// a class that would discard it anyway does not excuse a value outside + /// the documented range. + #[test] + fn a_level_the_class_discards_is_still_range_checked() { + new_ucmd!() + .args(&["-c", "3", "-n", "9", "true"]) + .fails() + .code_is(1) + .no_stdout() + .stderr_is("ionice: invalid class data argument: '9': must be 0-7\n"); + } + + /// ionice(1) documents -c as 0 to 3. The reference warns `unknown prio + /// class N` and carries on regardless, letting the kernel take the class + /// from three bits: `-c 10` runs best-effort and `-c 99` runs idle, both + /// exiting 0, while `-c 4` wraps onto a class the kernel refuses. Only -t + /// makes the wrap silent. This port rejects all three - see + /// uutils/util-linux#624. + #[test] + fn the_class_is_range_checked() { + for class in ["4", "10", "99"] { + new_ucmd!() + .args(&["-c", class, "true"]) + .fails() + .code_is(1) + .no_stdout() + .stderr_is(format!( + "ionice: invalid class argument: '{class}': must be 0-3\n" + )); + } + } + + /// -t silences a failure to set a priority, not a refusal to accept the + /// argument: a rejected value never reaches a syscall. + #[test] + fn ignore_does_not_silence_a_rejected_value() { + for (arguments, message) in [ + ( + ["-t", "-c", "99", "true"], + "invalid class argument: '99': must be 0-3", + ), + ( + ["-t", "-n", "99", "true"], + "invalid class data argument: '99': must be 0-7", + ), + ] { + new_ucmd!() + .args(&arguments) + .fails() + .code_is(1) + .no_stdout() + .stderr_is(format!("ionice: {message}\n")); + } + } + + /// -t keeps a refused set from stopping the command. Realtime is the one + /// class an unprivileged caller cannot have, which is what makes the set + /// fail here. + #[test] + fn ignore_silences_a_failed_set_and_still_runs_the_command() { + if skipped_as_root() { + return; + } + new_ucmd!() + .args(&["-t", "-c", "1", "echo", "ok"]) + .succeeds() + .no_stderr() + .stdout_is("ok\n"); + } + + #[test] + fn long_options_are_accepted() { + nested(&["--class=idle"]) + .succeeds() + .no_stderr() + .stdout_is("idle\n"); + nested(&["--class", "2", "--classdata", "3"]) + .succeeds() + .no_stderr() + .stdout_is("best-effort: prio 3\n"); + } + + // -- setting a priority by id -------------------------------------------- + + /// The word ioprio_get returns for a process an idle set was applied to, + /// spelled out from the kernel ABI so that the assertion shares nothing + /// with the tool's own encoder: class 3 above the 13-bit level field. The + /// level is 7 because that is what the reference packs with an idle set, + /// observed under strace; stdout cannot see it, because an idle priority + /// prints as the bare word "idle" whatever level it carries. + const IDLE_WORD: libc::c_long = (3 << 13) | 7; + + /// Read the raw priority word with a direct syscall, bypassing the code + /// under test. + fn raw_ioprio_of(pid: u32) -> libc::c_long { + const IOPRIO_WHO_PROCESS: libc::c_long = 1; + // SAFETY: ioprio_get takes two integers by value and touches no memory. + unsafe { + libc::syscall( + libc::SYS_ioprio_get, + IOPRIO_WHO_PROCESS, + pid as libc::c_long, + ) + } + } + + /// A set is applied to the option's id and to every trailing id, and it + /// reports nothing. + #[test] + fn a_set_applies_to_every_given_id() { + let mut children: Vec = (0..2) + .map(|_| { + Command::new("sleep") + .arg("10") + .spawn() + .expect("spawn sleep") + }) + .collect(); + let ids: Vec = children + .iter() + .map(|child| child.id().to_string()) + .collect(); + + new_ucmd!() + .args(&["-c", "3", "-p", ids[0].as_str(), ids[1].as_str()]) + .succeeds() + .no_output(); + + let words: Vec<_> = children + .iter() + .map(|child| raw_ioprio_of(child.id())) + .collect(); + + for child in &mut children { + child.kill().expect("kill sleep"); + child.wait().expect("reap sleep"); + } + + assert_eq!(words, [IDLE_WORD, IDLE_WORD]); + } + + // -- rejecting bad arguments --------------------------------------------- + + #[test] + fn rejects_an_unknown_class_name() { + for value in ["bogus", "", " 3"] { + new_ucmd!() + .args(&["-c", value]) + .fails() + .code_is(1) + .stderr_is(format!("ionice: unknown scheduling class: '{value}'\n")); + } + } + + /// A leading digit selects the numeric form, so a leading blank and a + /// trailing blank are reported differently. + #[test] + fn rejects_a_malformed_class_number() { + for value in ["0x3", "3 ", "3x"] { + new_ucmd!() + .args(&["-c", value]) + .fails() + .code_is(1) + .stderr_is(format!("ionice: invalid class argument: '{value}'\n")); + } + } + + /// A signed number has no leading digit, so it lands in the name branch. + #[test] + fn a_signed_class_number_is_taken_for_a_name() { + new_ucmd!() + .args(&["-c", "+3"]) + .fails() + .code_is(1) + .stderr_is("ionice: unknown scheduling class: '+3'\n"); + } + + /// A value too wide for an i32 is reported as such, ahead of the + /// documented range: that failure is the reference's own. + #[test] + fn rejects_an_oversized_class_number() { + new_ucmd!() + .args(&["-c", "4294967296"]) + .fails() + .code_is(1) + .stderr_contains("ionice: invalid class argument: '4294967296': ") + .stderr_does_not_contain("must be "); + } + + /// Every numeric option, not only -c, attaches a detail to an oversized + /// value, and it is the overflow rather than the documented range. + #[test] + fn an_oversized_number_carries_a_detail_on_every_option() { + for (option, name) in [ + ("-n", "class data"), + ("-p", "PID"), + ("-P", "PGID"), + ("-u", "UID"), + ] { + new_ucmd!() + .args(&[option, "4294967296"]) + .fails() + .code_is(1) + .stderr_contains(format!("ionice: invalid {name} argument: '4294967296': ")) + .stderr_does_not_contain("must be "); + } + } + + #[test] + fn rejects_a_malformed_level() { + for value in ["bogus", "0x3", "5 ", ""] { + new_ucmd!() + .args(&["-n", value]) + .fails() + .code_is(1) + .stderr_is(format!("ionice: invalid class data argument: '{value}'\n")); + } + } + + /// Unlike -c, -n has no name form, so a leading blank reaches the number + /// parser and is skipped. + #[test] + fn a_level_may_carry_leading_blanks() { + nested(&["-n", " 5"]) + .succeeds() + .no_stderr() + .stdout_is("best-effort: prio 5\n"); + } + + /// The number parser takes an optional sign, so +5 is level 5. + #[test] + fn a_level_may_carry_a_plus_sign() { + nested(&["-n", "+5"]) + .succeeds() + .no_stderr() + .stdout_is("best-effort: prio 5\n"); + } + + #[test] + fn rejects_a_malformed_id() { + for (option, name) in [("-p", "PID"), ("-P", "PGID"), ("-u", "UID")] { + new_ucmd!() + .args(&[option, "bogus"]) + .fails() + .code_is(1) + .stderr_is(format!("ionice: invalid {name} argument: 'bogus'\n")); + } + } + + #[test] + fn does_not_resolve_user_names() { + new_ucmd!() + .args(&["-u", "root"]) + .fails() + .code_is(1) + .stderr_is("ionice: invalid UID argument: 'root'\n"); + } + + /// The value after an id option is taken verbatim, even when it looks like + /// another option. + #[test] + fn an_id_value_may_start_with_a_hyphen() { + new_ucmd!() + .args(&["-p", "-c", "3"]) + .fails() + .code_is(1) + .stderr_is("ionice: invalid PID argument: '-c'\n"); + } + + #[test] + fn only_one_kind_of_id_at_a_time() { + for arguments in [ + ["-p", "1", "-P", "1"], + ["-p", "1", "-u", "0"], + ["-p", "1", "-p", "2"], + ] { + new_ucmd!() + .args(&arguments) + .fails() + .code_is(1) + .stderr_is("ionice: can handle only one of pid, pgid or uid at once\n"); + } + } + + /// The options are judged in the order they were written, so it is always + /// the leftmost bad one that is reported. + #[test] + fn the_first_bad_option_is_the_one_reported() { + new_ucmd!() + .args(&["-P", "1", "-p", "bogus"]) + .fails() + .code_is(1) + .stderr_is("ionice: can handle only one of pid, pgid or uid at once\n"); + + new_ucmd!() + .args(&["-p", "bogus", "-P", "1"]) + .fails() + .code_is(1) + .stderr_is("ionice: invalid PID argument: 'bogus'\n"); + + new_ucmd!() + .args(&["-n", "8", "-p", "bogus"]) + .fails() + .code_is(1) + .stderr_is("ionice: invalid class data argument: '8': must be 0-7\n"); + + new_ucmd!() + .args(&["-c", "bogus", "-p", "bogus"]) + .fails() + .code_is(1) + .stderr_is("ionice: unknown scheduling class: 'bogus'\n"); + } + + #[test] + fn a_priority_with_nothing_to_apply_it_to_is_a_usage_error() { + for arguments in [vec!["-c", "3"], vec!["-n", "5"], vec!["-t", "-c", "3"]] { + new_ucmd!() + .args(&arguments) + .fails() + .code_is(1) + .no_stdout() + .stderr_is("ionice: bad usage\nTry 'ionice --help' for more information.\n"); + } + } + + /// The options are resolved before the missing target is noticed, so a + /// rejected value is the only thing reported. + #[test] + fn a_rejected_class_preempts_the_usage_error() { + new_ucmd!() + .args(&["-c", "4"]) + .fails() + .code_is(1) + .no_stdout() + .stderr_is("ionice: invalid class argument: '4': must be 0-3\n"); + } + + /// A warning keeps that ordering too: the priority is settled, and says + /// what it discarded, before the usage error. + #[test] + fn the_dropped_level_warning_comes_before_the_usage_error() { + new_ucmd!() + .args(&["-c", "3", "-n", "5"]) + .fails() + .code_is(1) + .no_stdout() + .stderr_is( + "ionice: ignoring given class data for idle class\nionice: bad usage\n\ + Try 'ionice --help' for more information.\n", + ); + } + + /// Argument bytes that are not valid Unicode must reach the utility, so + /// that a bad value is diagnosed here rather than refused by the parser + /// before ionice sees it at all. Only the diagnosis is asserted; the value + /// echoed back is rendered lossily, which is a divergence of its own. + #[test] + fn an_option_value_that_is_not_valid_unicode_is_diagnosed_here() { + new_ucmd!() + .arg("-p") + .arg(OsStr::from_bytes(b"1\xff")) + .fails() + .code_is(1) + .stderr_contains("invalid PID argument: "); + } + + // -- running a command --------------------------------------------------- + + #[test] + fn arguments_after_the_command_belong_to_the_command() { + new_ucmd!() + .args(&["-c", "3", "echo", "-p", "5"]) + .succeeds() + .stdout_is("-p 5\n"); + + new_ucmd!() + .args(&["echo", "-c", "3"]) + .succeeds() + .stdout_is("-c 3\n"); + + new_ucmd!() + .args(&["-c", "3", "--", "echo", "-c"]) + .succeeds() + .stdout_is("-c\n"); + } + + #[test] + fn the_command_exit_status_is_passed_through() { + new_ucmd!() + .args(&["-c", "3", "sh", "-c", "exit 42"]) + .fails() + .code_is(42) + .no_output(); + } + + #[test] + fn a_command_that_does_not_exist_fails() { + new_ucmd!() + .args(&["-c", "3", "this-command-does-not-exist-hopefully"]) + .fails() + .code_is(127) + .stderr_is( + "ionice: failed to execute this-command-does-not-exist-hopefully: \ + No such file or directory\n", + ); + } + + #[test] + fn a_command_that_cannot_be_executed_fails() { + new_ucmd!() + .args(&["-c", "3", "."]) + .fails() + .code_is(126) + .stderr_is("ionice: failed to execute .: Permission denied\n"); + } + + #[test] + fn ignore_does_not_silence_a_failed_exec() { + new_ucmd!() + .args(&["-t", "this-command-does-not-exist-hopefully"]) + .fails() + .code_is(127) + .stderr_contains("failed to execute"); + } + + /// The bytes of a command name must reach exec unchanged. A directory is + /// the discriminator: exec refuses one with EACCES, so a 126 proves the + /// name arrived intact, where bytes flattened into replacement characters + /// would have named nothing and failed with 127. + #[test] + fn a_command_name_that_is_not_valid_unicode_reaches_exec_unchanged() { + let (at, mut ucmd) = at_and_ucmd!(); + let name = OsStr::from_bytes(b"not-\xffunicode"); + std::fs::create_dir(at.plus(name)).unwrap(); + + ucmd.args(&["-c", "3"]) + .arg(at.plus(name)) + .fails() + .code_is(126) + .stderr_contains("Permission denied"); + } + + // -- privilege ----------------------------------------------------------- + + /// The tests that call this expect a set to be refused, which holds only + /// while the caller lacks the privilege to make it succeed: root may take + /// the realtime class, and root setting idle on pid 1 really does re-class + /// init rather than earning the EPERM the test expects. Two cases this + /// does not cover, both absent from CI: a user namespace whose pid 1 the + /// caller owns, and a caller carrying CAP_SYS_NICE. + fn skipped_as_root() -> bool { + if unsafe { libc::geteuid() } == 0 { + println!("test skipped: running as root would let the set succeed"); + return true; + } + false + } + + /// pid 1 belongs to root, and it is that ownership - not the target being + /// another process - that earns the EPERM: a caller may lower a process it + /// owns, as `a_set_applies_to_every_given_id` shows. + #[test] + fn a_set_on_a_process_owned_by_another_user_is_refused() { + if skipped_as_root() { + return; + } + new_ucmd!() + .args(&["-c", "3", "-p", "1"]) + .fails() + .code_is(1) + .no_stdout() + .stderr_is("ionice: ioprio_set failed: Operation not permitted\n"); + } + + #[test] + fn ignore_silences_a_refused_set() { + if skipped_as_root() { + return; + } + new_ucmd!() + .args(&["-t", "-c", "3", "-p", "1"]) + .succeeds() + .no_output(); + } +} + +#[cfg(not(target_os = "linux"))] +mod non_linux { + use uutests::new_ucmd; + + #[test] + fn fails_on_unsupported_platforms() { + new_ucmd!() + .args(&["-p", "1"]) + .fails() + .code_is(1) + .stderr_is("ionice: `ionice` is available only on Linux.\n"); + } +} diff --git a/tests/tests.rs b/tests/tests.rs index 95682dd3..02c73428 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -98,3 +98,7 @@ mod test_uuidgen; #[cfg(feature = "chcpu")] #[path = "by-util/test_chcpu.rs"] mod test_chcpu; + +#[cfg(feature = "ionice")] +#[path = "by-util/test_ionice.rs"] +mod test_ionice; From 052d91cfb14c94ee1430fcf858de08782e4640bf Mon Sep 17 00:00:00 2001 From: mmc Date: Sat, 29 Aug 2026 13:11:16 -0500 Subject: [PATCH 3/3] ionice: correct two comments about the idle class Both said the idle class has no priority level. It has one; ionice just does not print it. The kernel takes whatever level is packed with an idle set and hands it back unchanged - fixed_data packs 7 with every idle set, and the integration tests assert that ioprio_get returns (3 << 13) | 7 for a process one was applied to, which is only possible because the level survives. The wording mattered for the next person to touch either place: told the kernel ignores the level, they would read the 7 as arbitrary and the read-back assertion as pointless, and weakening that assertion would remove the only check on our idle encoding, since stdout prints the bare word "idle" whatever level it carries. The claim behind the choice of 7 is unchanged and still stands: nothing in the kernel forces it, so it is the reference's convention that we follow. Comment text only; no code line moved. --- src/uu/ionice/src/ionice.rs | 3 ++- src/uu/ionice/src/ioprio.rs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/uu/ionice/src/ionice.rs b/src/uu/ionice/src/ionice.rs index 3d6256a7..d58f6fd3 100644 --- a/src/uu/ionice/src/ionice.rs +++ b/src/uu/ionice/src/ionice.rs @@ -177,7 +177,8 @@ mod linux { /// The two arms have different authorities behind them. None must carry /// zero because that is the only value the kernel accepts for it. Idle /// carrying 7 is the reference implementation's convention: the kernel - /// ignores idle's level entirely, so nothing forces that choice. + /// takes any level with idle and hands it back unchanged, so nothing + /// forces that choice. fn fixed_data(class: i32) -> Option { match class { ioprio::IOPRIO_CLASS_NONE => Some(0), diff --git a/src/uu/ionice/src/ioprio.rs b/src/uu/ionice/src/ioprio.rs index 792f1916..459c08e6 100644 --- a/src/uu/ionice/src/ioprio.rs +++ b/src/uu/ionice/src/ioprio.rs @@ -61,7 +61,8 @@ impl fmt::Display for IoPrio { let class = self.class(); if class == IOPRIO_CLASS_IDLE { - // The idle class carries no priority level. + // An idle priority prints as the bare class name; the level it + // carries is not shown. f.write_str(class_name(class)) } else { write!(f, "{}: prio {}", class_name(class), self.data())