Skip to content

chroot: remove unwrap calls#7890

Merged
sylvestre merged 4 commits into
uutils:mainfrom
jovielarue:fix-unwrap
May 28, 2025
Merged

chroot: remove unwrap calls#7890
sylvestre merged 4 commits into
uutils:mainfrom
jovielarue:fix-unwrap

Conversation

@jovielarue

Copy link
Copy Markdown
Contributor

This PR aims to solve #7876. I have not found the underlying problem to that issue, but I have removed the unwrap() calls on lines 442 and 451 as mentioned in #7876 so that the code aligns with the "never use panic!" guideline.

@github-actions

github-actions Bot commented May 6, 2025

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/misc/usage_vs_getopt (passes in this run but fails in the 'main' branch)

Comment thread src/uu/chroot/src/chroot.rs Outdated
CString::new(root.as_os_str().as_bytes().to_vec())
.unwrap()
.map_err(|e| {
ChrootError::CannotEnter("Unable to enter root directory".to_string(), e.into())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i don't think the "unable to enter..." string should be passed

#[error("cannot chroot to {dir}: {err}", dir = .0.quote(), err = .1)]

the first arg is the path (so, root here)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Got it, I'll fix that. Sorry for the delay, I've been on vacation!

@sylvestre

Copy link
Copy Markdown
Contributor

@jovielarue ping ? :)

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@sylvestre sylvestre merged commit 0f632f2 into uutils:main May 28, 2025
72 of 73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants