uucore: os_str_from_bytes does not need to alloc

[lhecker]

The borrow checker agrees: 9 out of 10 doctors
recommend os_str_from_bytes to be alloc free.

[Copilot]

🟡 Changes recommended

It introduces a semver-breaking public API change and lacks targeted tests covering the platform-dependent behavior and new borrowing semantics.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates uucore::os_str_from_bytes to avoid allocations by returning a borrowed &OsStr directly, and adjusts internal call sites accordingly.

Changes:

• Changed os_str_from_bytes return type from UResult<Cow<'_, OsStr>> to UResult<&OsStr> and removed owned conversion on non-unix.
• Updated call sites to use to_owned() / direct &OsStr passing instead of into_owned() / extra deref.

File summaries

File	Description
src/uucore/src/lib/lib.rs	Changes os_str_from_bytes signature/implementation to be alloc-free and updates docs accordingly.
src/uucore/src/lib/features/fsext.rs	Adapts mount-dir conversion to the new borrowed return type.
src/uucore/src/lib/features/diagnostics.rs	Adjusts argument conversion to match the new return type.
src/uucore/src/lib/features/checksum/validate.rs	Updates checksum validation path handling to pass &OsStr directly.
src/uu/du/src/du.rs	Removes unnecessary deref when converting byte-paths to PathBuf.

Review details

Suppressed comments (3)

src/uucore/src/lib/lib.rs:513

• The error message produced on non-unix is vague about the actual failure mode. Since the failure is specifically from_utf8 rejecting invalid UTF-8, the message should mention UTF-8 to make the diagnosis actionable.

    Ok(OsStr::new(str::from_utf8(bytes).map_err(|_| {
        error::UUsageError::new(1, "Unable to transform bytes into OsStr")
    })?))

src/uucore/src/lib/lib.rs:509

• This changes os_str_from_bytes’s public return type from UResult<Cow<'_, OsStr>> to UResult<&OsStr>, which is a semver-breaking API change for the uucore crate. If the goal is only to remove allocations, consider keeping the old Cow-based signature (returning Cow::Borrowed on all platforms) or providing a new alloc-free API alongside the existing one and deprecating the old later.

pub fn os_str_from_bytes(bytes: &[u8]) -> error::UResult<&OsStr> {
    #[cfg(any(unix, all(target_os = "wasi", target_env = "p1")))]
    return Ok(OsStr::from_bytes(bytes));

src/uucore/src/lib/lib.rs:513

• This PR changes the API and borrowing semantics of os_str_from_bytes, but there are no unit tests covering the key platform-dependent behaviors (invalid UTF-8 should succeed on unix and error on non-unix; valid UTF-8 should round-trip). Project guidelines require a test for behavior changes to prevent regressions.

pub fn os_str_from_bytes(bytes: &[u8]) -> error::UResult<&OsStr> {
    #[cfg(any(unix, all(target_os = "wasi", target_env = "p1")))]
    return Ok(OsStr::from_bytes(bytes));

    #[cfg(not(any(unix, all(target_os = "wasi", target_env = "p1"))))]
    Ok(OsStr::new(str::from_utf8(bytes).map_err(|_| {
        error::UUsageError::new(1, "Unable to transform bytes into OsStr")
    })?))

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/misc/io-errors (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/misc/tty-eof (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/retry (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/symlink (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/pr/bounded-memory (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/cp/link-heap is now being skipped but was previously passing.

[sylvestre]

But the ci is sad

[lhecker]

Ah right, I had to run clippy for all features and projects...

[Copilot]

🟡 Changes recommended

It introduces a semver-breaking public uucore API signature change that needs an explicit compatibility plan (e.g., keep/deprecate old API or coordinate a major version bump).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 1
• Review effort level: Lite