[brad0]

OpenBSD added getexecpath(3) to libc, which returns an
already-canonicalized executable path directly. Replace the
sysctl(KERN_PROC_ARGV)-based implementation.

This was the last user of div_exact in std, so drop the
exact_div feature gate.

[rustbot]

r? @JohnTitor

rustbot has assigned @JohnTitor.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: @ChrisDenton, libs
• @ChrisDenton, libs expanded to 13 candidates
• Random selection from 7 candidates

[brad0]

Just waiting for libc 0.2.191.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[brad0]

@semarie

[semarie]

Just a question : any particular reason to diverge from the patch currently used downstream ? see lang/rust/patches/patch-library_std_src_sys_paths_unix_rs

[kpcyrd]

@semarie from how I'm reading the code, the downstream code does one extra heap allocation.

The downstream one does:

• allocate libc::PATH_MAX bytes on the heap (len == capacity)
• lets the kernel write data to this address
• takes a slice from that heap allocation (from address to first null byte)
• uses OsStr::from_bytes to get a "Borrowed reference to an OS string" (which has a life-time dependency on the first heap allocation)
• uses PathBuf::from with From<OsStr> to copy the bytes from that slice into a new, second heap allocation (resolving the life-time dependency on the first allocation, which can now be safely freed)

The one suggested here does:

• allocate libc::PATH_MAX bytes on the heap (len == capacity)
• lets the kernel write data to this address
• counts the bytes until the first null byte and then adjusts len accordingly
• uses OsString::from_vec to pass ownership of the vec to an OsString, which owns it's own data, without a life-time dependency
• uses PathBuf::from with From<OsString> to pass ownership of that OsString to the PathBuf

Due to type-erasure during compile, the last two steps in the later approach are effectively no-ops at runtime, if I'm not mistaken.

[catap]

Probably I've missed why do we need truncate here. I think it should be as simple as:

    let mut path = vec![0; libc::PATH_MAX as usize];
    unsafe {
        cvt(getexecpath(path.as_mut_ptr(), path.len()))?;
        let path = CStr::from_ptr(path.as_ptr()).to_bytes();
        Ok(PathBuf::from(OsStr::from_bytes(path)))
    }

View changes since the review

[kpcyrd]

This code does two heap allocations instead of just one, see this comment: #163393 (comment)

[brad0]

• It uses the libc binding instead of a local extern block.
• It avoids a second allocation: the buffer is truncated at the NUL
  and handed to OsString::from_vec, rather than borrowing it as an
  &OsStr and copying it into a new PathBuf. This matches the
  FreeBSD/NetBSD/Apple current_exe() implementations in the
  same file.
• With the sysctl/argv code gone, nothing in std uses div_exact any
  more, so #![feature(exact_div)] is dropped.

[semarie]

LGTM.
thanks

View changes since this review

[brad0]

@JohnTitor Ping.

[JohnTitor]

Sorry for the delay! LGTM @bors r+

View changes since this review

[rust-bors]

📌 Commit 5fbbf47 has been approved by JohnTitor

It is now in the queue for this repository.

[brad0]

@JohnTitor Thanks!