Skip to content

Id: don't panic on write error #10769

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

Merged
ChrisDryden merged 8 commits into from
Feb 6, 2026
Merged

Id: don't panic on write error #10769

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1569267
Merge pull request #2 from uutils/main
FidelSch Jan 28, 2026
5e506cc
Merge https://github.com/uutils/coreutils
FidelSch Jan 29, 2026
f67556c
id: replace print statements with write to stdout lock for error hand…
FidelSch Feb 6, 2026
2babf04
id: replace print statements with write to stdout lock for error hand…
FidelSch Feb 6, 2026
1070338
Merge branch 'id-panic-on-write' of https://github.com/FidelSch/coreu…
FidelSch Feb 6, 2026
ff093a7
id: remove unwrap where possible
FidelSch Feb 6, 2026
d12150e
id: lint
FidelSch Feb 6, 2026
2049d83
id: handle errors in write operations for SELinux and SMACK labels
FidelSch Feb 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 88 additions & 60 deletions src/uu/id/src/id.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

use clap::{Arg, ArgAction, Command};
use std::ffi::CStr;
use std::io::{self, Write};
use uucore::display::Quotable;
use uucore::entries::{self, Group, Locate, Passwd};
use uucore::error::UResult;
Expand Down Expand Up @@ -124,6 +125,7 @@ struct State {
#[allow(clippy::cognitive_complexity)]
pub fn uumain(args: impl uucore::Args) -> UResult<()> {
let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?;
let mut lock = io::stdout().lock();
Copy link
Copy Markdown
Collaborator

@ChrisDryden ChrisDryden Feb 6, 2026

Choose a reason for hiding this comment

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

I'm taking a first read through and everything seems right, I need to do some more local testing because I am realizing I don't on a deep level understand what would be better, locking it at the beginning of locking it for each write

Copy link
Copy Markdown
Collaborator

@ChrisDryden ChrisDryden Feb 6, 2026

Choose a reason for hiding this comment

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

So negligible measured perf differences, in theory on write heavy utilities this could have performance implications so this would be the right approach. We just have to be careful on multi-threaded utilities

Copy link
Copy Markdown
Contributor Author

@FidelSch FidelSch Feb 6, 2026

Choose a reason for hiding this comment

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

As I understand from this, "The print! macro will lock the standard output on each call. If you call print! within a hot loop, this behavior may be the bottleneck of the loop. To avoid this, lock stdout with io::stdout().lock()", locking and unlocking seems to incur some overhead, that's why I opted to lock once.

Copy link
Copy Markdown
Contributor

@oech3 oech3 Feb 6, 2026

Choose a reason for hiding this comment

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

Or running many utils at same terminal?


let users: Vec<String> = matches
.get_many::<String>(options::ARG_USERS)
Expand Down Expand Up @@ -181,7 +183,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
if state.selinux_supported {
if let Ok(context) = selinux::SecurityContext::current(false) {
let bytes = context.as_bytes();
print!("{}{line_ending}", String::from_utf8_lossy(bytes));
write!(lock, "{}{line_ending}", String::from_utf8_lossy(bytes))?;
return Ok(());
}
return Err(USimpleError::new(
Expand All @@ -195,7 +197,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
if state.smack_supported {
match uucore::smack::get_smack_label_for_self() {
Ok(label) => {
print!("{label}{line_ending}");
write!(lock, "{label}{line_ending}")?;
return Ok(());
}
Err(_) => {
Expand Down Expand Up @@ -240,17 +242,17 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
// GNU's `id` does not support the flags: -p/-P/-A.
if matches.get_flag(options::OPT_PASSWORD) {
// BSD's `id` ignores all but the first specified user
pline(possible_pw.as_ref().map(|v| v.uid));
pline(possible_pw.as_ref().map(|v| v.uid))?;
return Ok(());
}
if matches.get_flag(options::OPT_HUMAN_READABLE) {
// BSD's `id` ignores all but the first specified user
pretty(possible_pw);
pretty(possible_pw)?;
return Ok(());
}
if matches.get_flag(options::OPT_AUDIT) {
// BSD's `id` ignores specified users
auditid();
auditid()?;
return Ok(());
}

Expand All @@ -273,7 +275,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
});

if state.gflag {
print!(
write!(
lock,
"{}",
if state.nflag {
entries::gid2grp(gid).unwrap_or_else(|_| {
Expand All @@ -287,11 +290,12 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
} else {
gid.to_string()
}
);
)?;
}

if state.uflag {
print!(
write!(
lock,
"{}",
if state.nflag {
entries::uid2usr(uid).unwrap_or_else(|_| {
Expand All @@ -305,18 +309,19 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
} else {
uid.to_string()
}
);
)?;
}

let groups = entries::get_groups_gnu(Some(gid)).unwrap();
let groups = entries::get_groups_gnu(Some(gid))?;
let groups = if state.user_specified {
possible_pw.as_ref().map(|p| p.belongs_to()).unwrap()
} else {
groups.clone()
};

if state.gsflag {
print!(
write!(
lock,
"{}{}",
groups
.iter()
Expand All @@ -342,13 +347,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
} else {
""
}
);
)?;
}

if default_format {
id_print(&state, &groups);
id_print(&state, &groups)?;
}
print!("{line_ending}");
write!(lock, "{line_ending}")?;

if i + 1 >= users.len() {
break;
Expand Down Expand Up @@ -469,72 +474,79 @@ pub fn uu_app() -> Command {
)
}

fn pretty(possible_pw: Option<Passwd>) {
fn pretty(possible_pw: Option<Passwd>) -> io::Result<()> {
let mut lock = io::stdout().lock();

if let Some(p) = possible_pw {
print!(
writeln!(
lock,
"{}\t{}\n{}\t",
translate!("id-output-uid"),
p.name,
translate!("id-output-groups")
);
println!(
)?;
writeln!(
lock,
"{}",
p.belongs_to()
.iter()
.map(|&gr| entries::gid2grp(gr).unwrap_or_else(|_| gr.to_string()))
.collect::<Vec<_>>()
.join(" ")
);
)?;
} else {
let login = cstr2cow!(getlogin().cast_const());
let uid = getuid();
if let Ok(p) = Passwd::locate(uid) {
if let Some(user_name) = login {
println!("{}\t{user_name}", translate!("id-output-login"));
writeln!(lock, "{}\t{user_name}", translate!("id-output-login"))?;
}
println!("{}\t{}", translate!("id-output-uid"), p.name);
writeln!(lock, "{}\t{}", translate!("id-output-uid"), p.name)?;
} else {
println!("{}\t{uid}", translate!("id-output-uid"));
writeln!(lock, "{}\t{uid}", translate!("id-output-uid"))?;
}

let euid = geteuid();
if euid != uid {
if let Ok(p) = Passwd::locate(euid) {
println!("{}\t{}", translate!("id-output-euid"), p.name);
writeln!(lock, "{}\t{}", translate!("id-output-euid"), p.name)?;
} else {
println!("{}\t{euid}", translate!("id-output-euid"));
writeln!(lock, "{}\t{euid}", translate!("id-output-euid"))?;
}
}

let rgid = getgid();
let egid = getegid();
if egid != rgid {
if let Ok(g) = Group::locate(rgid) {
println!("{}\t{}", translate!("id-output-rgid"), g.name);
writeln!(lock, "{}\t{}", translate!("id-output-rgid"), g.name)?;
} else {
println!("{}\t{rgid}", translate!("id-output-rgid"));
writeln!(lock, "{}\t{rgid}", translate!("id-output-rgid"))?;
}
}

println!(
writeln!(
lock,
"{}\t{}",
translate!("id-output-groups"),
entries::get_groups_gnu(None)
.unwrap()
entries::get_groups_gnu(None)?
.iter()
.map(|&gr| entries::gid2grp(gr).unwrap_or_else(|_| gr.to_string()))
.collect::<Vec<_>>()
.join(" ")
);
)?;
}

Ok(())
}

#[cfg(any(target_vendor = "apple", target_os = "freebsd"))]
fn pline(possible_uid: Option<uid_t>) {
fn pline(possible_uid: Option<uid_t>) -> io::Result<()> {
let uid = possible_uid.unwrap_or_else(getuid);
let pw = Passwd::locate(uid).unwrap();
let pw = Passwd::locate(uid)?;

println!(
writeln!(
io::stdout().lock(),
"{}:{}:{}:{}:{}:{}:{}:{}:{}:{}",
pw.name,
pw.user_passwd.unwrap_or_default(),
Expand All @@ -546,7 +558,7 @@ fn pline(possible_uid: Option<uid_t>) {
pw.user_info.unwrap_or_default(),
pw.user_dir.unwrap_or_default(),
pw.user_shell.unwrap_or_default()
);
)
}

#[cfg(any(
Expand All @@ -555,11 +567,12 @@ fn pline(possible_uid: Option<uid_t>) {
target_os = "openbsd",
target_os = "cygwin"
))]
fn pline(possible_uid: Option<uid_t>) {
fn pline(possible_uid: Option<uid_t>) -> io::Result<()> {
let uid = possible_uid.unwrap_or_else(getuid);
let pw = Passwd::locate(uid).unwrap();
let pw = Passwd::locate(uid)?;

println!(
writeln!(
io::stdout().lock(),
"{}:{}:{}:{}:{}:{}:{}",
pw.name,
pw.user_passwd.unwrap_or_default(),
Expand All @@ -568,7 +581,8 @@ fn pline(possible_uid: Option<uid_t>) {
pw.user_info.unwrap_or_default(),
pw.user_dir.unwrap_or_default(),
pw.user_shell.unwrap_or_default()
);
)?;
Ok(())
}

#[cfg(any(
Expand All @@ -577,41 +591,49 @@ fn pline(possible_uid: Option<uid_t>) {
target_os = "openbsd",
target_os = "cygwin"
))]
fn auditid() {}
#[allow(clippy::unnecessary_wraps)]
fn auditid() -> io::Result<()> {
Ok(())
}

#[cfg(not(any(
target_os = "linux",
target_os = "android",
target_os = "openbsd",
target_os = "cygwin"
)))]
fn auditid() {
fn auditid() -> io::Result<()> {
use std::mem::MaybeUninit;
let mut lock = io::stdout().lock();

let mut auditinfo: MaybeUninit<audit::c_auditinfo_addr_t> = MaybeUninit::uninit();
let address = auditinfo.as_mut_ptr();
if unsafe { audit::getaudit(address) } < 0 {
println!("{}", translate!("id-error-audit-retrieve"));
return;
writeln!(lock, "{}", translate!("id-error-audit-retrieve"))?;
return Ok(());
}

// SAFETY: getaudit wrote a valid struct to auditinfo
let auditinfo = unsafe { auditinfo.assume_init() };

println!("auid={}", auditinfo.ai_auid);
println!("mask.success=0x{:x}", auditinfo.ai_mask.am_success);
println!("mask.failure=0x{:x}", auditinfo.ai_mask.am_failure);
println!("termid.port=0x{:x}", auditinfo.ai_termid.port);
println!("asid={}", auditinfo.ai_asid);
writeln!(lock, "auid={}", auditinfo.ai_auid)?;
writeln!(lock, "mask.success=0x{:x}", auditinfo.ai_mask.am_success)?;
writeln!(lock, "mask.failure=0x{:x}", auditinfo.ai_mask.am_failure)?;
writeln!(lock, "termid.port=0x{:x}", auditinfo.ai_termid.port)?;
writeln!(lock, "asid={}", auditinfo.ai_asid)?;
Ok(())
}

fn id_print(state: &State, groups: &[u32]) {
fn id_print(state: &State, groups: &[u32]) -> io::Result<()> {
let uid = state.ids.as_ref().unwrap().uid;
let gid = state.ids.as_ref().unwrap().gid;
let euid = state.ids.as_ref().unwrap().euid;
let egid = state.ids.as_ref().unwrap().egid;

print!(
let mut lock = io::stdout().lock();

write!(
lock,
"uid={uid}({})",
entries::uid2usr(uid).unwrap_or_else(|_| {
show_error!(
Expand All @@ -621,8 +643,9 @@ fn id_print(state: &State, groups: &[u32]) {
set_exit_code(1);
uid.to_string()
})
);
print!(
)?;
write!(
lock,
" gid={gid}({})",
entries::gid2grp(gid).unwrap_or_else(|_| {
show_error!(
Expand All @@ -632,9 +655,10 @@ fn id_print(state: &State, groups: &[u32]) {
set_exit_code(1);
gid.to_string()
})
);
)?;
if !state.user_specified && (euid != uid) {
print!(
write!(
lock,
" euid={euid}({})",
entries::uid2usr(euid).unwrap_or_else(|_| {
show_error!(
Expand All @@ -644,11 +668,12 @@ fn id_print(state: &State, groups: &[u32]) {
set_exit_code(1);
euid.to_string()
})
);
)?;
}
if !state.user_specified && (egid != gid) {
// BUG? printing egid={euid} ?
print!(
write!(
lock,
" egid={egid}({})",
entries::gid2grp(egid).unwrap_or_else(|_| {
show_error!(
Expand All @@ -658,9 +683,10 @@ fn id_print(state: &State, groups: &[u32]) {
set_exit_code(1);
egid.to_string()
})
);
)?;
}
print!(
write!(
lock,
" groups={}",
groups
.iter()
Expand All @@ -677,7 +703,7 @@ fn id_print(state: &State, groups: &[u32]) {
))
.collect::<Vec<_>>()
.join(",")
);
)?;

#[cfg(feature = "selinux")]
if state.selinux_supported
Expand All @@ -687,7 +713,7 @@ fn id_print(state: &State, groups: &[u32]) {
// print SElinux context (does not depend on "-Z")
if let Ok(context) = selinux::SecurityContext::current(false) {
let bytes = context.as_bytes();
print!(" context={}", String::from_utf8_lossy(bytes));
write!(lock, " context={}", String::from_utf8_lossy(bytes))?;
}
}

Expand All @@ -698,9 +724,11 @@ fn id_print(state: &State, groups: &[u32]) {
{
// print SMACK label (does not depend on "-Z")
if let Ok(label) = uucore::smack::get_smack_label_for_self() {
print!(" context={label}");
write!(lock, " context={label}")?;
}
}

Ok(())
}

#[cfg(not(any(target_os = "linux", target_os = "android", target_os = "openbsd")))]
Expand Down
Loading