diff --git a/local/recipes/system/redbear-power/source/src/pid_detail.rs b/local/recipes/system/redbear-power/source/src/pid_detail.rs index b279813e02..d71a242858 100644 --- a/local/recipes/system/redbear-power/source/src/pid_detail.rs +++ b/local/recipes/system/redbear-power/source/src/pid_detail.rs @@ -111,29 +111,33 @@ fn read_cmdline(pid: u32) -> Option { } } -/// Read the I/O priority class from `/proc//stat` field -/// 47 (the priority is `class | (data << 13)`; we return the -/// raw field as u32 so the caller can interpret both). -/// Field 47 is the 47th space-separated token, 0-indexed at -/// the THIRD field (state) plus 44 (matches the kernel's -/// `getpriority(2)`-derived field). +/// Read the I/O priority (nice-equivalent) from +/// `/proc//stat`. The /proc/[pid]/stat format is +/// documented in `man 5 proc`; the priority field is at +/// position 18 (1-indexed), which is post-comm index 15 +/// (0-indexed `fields[15]`) because fields 1-2 are pid+comm +/// and field 3 is state. The v1.38 first cut used `fields[44]` +/// (overall field 47), which on modern Linux kernels (~52 +/// fields total) is a memory address (~9×10¹³) — not the +/// priority. Reading that value as u32 silently overflowed +/// and the reader returned None for every process. This +/// release fixes the field index. fn read_io_priority(pid: u32) -> Option { let stat = fs::read_to_string(format!("/proc/{pid}/stat")).ok()?; // The comm field is wrapped in parens and may contain // spaces, so we must find the LAST `)` to delimit the - // comm field, then split the rest by whitespace. The - // 44th post-comm field (1-indexed: field 47 overall) - // holds the priority. + // comm field, then split the rest by whitespace. After + // the comm: [0]=state [1]=ppid [2]=pgrp [3]=session + // [4]=tty_nr [5]=tpgid [6]=flags [7]=minflt [8]=cminflt + // [9]=majflt [10]=cmajflt [11]=utime [12]=stime + // [13]=cutime [14]=cstime [15]=priority. We want [15]. let close = stat.rfind(')')?; let tail = &stat[close + 1..]; let fields: Vec<&str> = tail.split_whitespace().collect(); - // post-comm field index 44 -> vec index 43 (0-indexed) - // post-comm field 1 is the state (after the comm), so - // 47th overall field is post-comm 45 (state + 44). - if fields.len() < 45 { + if fields.len() < 16 { return None; } - fields[44].parse::().ok() + fields[15].parse::().ok() } fn read_status(pid: u32) -> ProcStatus { @@ -312,10 +316,43 @@ mod tests { #[test] fn read_io_priority_handles_self() { - // Self's io_priority is a u32. Some kernels may not - // expose it; tolerate that. The function must not - // panic. - let _ = read_io_priority(std::process::id()); + // v1.38.1 regression test: the v1.38 first cut read + // `fields[44]` (overall field 47), which on modern + // Linux kernels is a memory address (~9×10¹³) that + // overflows u32 and silently returns None for every + // process. The fix reads `fields[15]` (overall field + // 18 = priority). This test reads the priority from + // /proc/self/stat directly (bypassing the function) + // and asserts the function returns the same value. + let pid = std::process::id(); + let stat = std::fs::read_to_string(format!("/proc/{pid}/stat")) + .expect("can read /proc/self/stat"); + let close = stat.rfind(')').expect("comm field has closing paren"); + let tail = &stat[close + 1..]; + let fields: Vec<&str> = tail.split_whitespace().collect(); + // 0-indexed: state=0, ppid=1, pgrp=2, session=3, + // tty_nr=4, tpgid=5, flags=6, minflt=7, cminflt=8, + // majflt=9, cmajflt=10, utime=11, stime=12, cutime=13, + // cstime=14, priority=15. + let expected: u32 = fields[15] + .parse() + .expect("priority is a u32"); + let actual = read_io_priority(pid) + .expect("read_io_priority must return Some for self"); + assert_eq!( + actual, expected, + "v1.38.1: read_io_priority should return the priority field (overall field 18, post-comm vec[15])" + ); + // Sanity: a typical default-nice process has priority + // 20. We're not strict (containers / reniced processes + // may differ), but if the value looks like a memory + // address (>10^9) we know we're reading the wrong + // field again. + assert!( + actual < 1_000_000_000, + "read_io_priority returned {} which looks like a memory address, not a priority. The field index is wrong again.", + actual + ); } #[test]