NetBSD: add sysctl backend for std::env::current_exe by jakllsch · Pull Request #46373 · rust-lang/rust

jakllsch · 2017-11-29T17:47:13Z

Use the CTL_KERN.KERN_PROC_ARGS.-1.KERN_PROC_PATHNAME sysctl in
preference over the /proc/curproc/exe symlink.

Additionally, perform more validation of aformentioned symlink.
Particularly on pre-8.x NetBSD this symlink will point to '/' when
accurate information is unavailable.

rust-highfive · 2017-11-29T17:47:18Z

r? @kennytm

(rust_highfive has picked a reviewer for you, use r? to override)

kennytm · 2017-11-29T17:51:14Z

Use sysctl().or_else(procfs) here, otherwise procfs() will always be called.

kennytm · 2017-11-29T17:52:09Z

Why a PathBuf instead of Path?

For no reason other than to match the linux,android,emscripten current_exe(). I had path::Path::new() in a previous version of this. I'll restore it.

kennytm · 2017-11-29T17:57:13Z

Do you need to check path_len == 0 just like the FreeBSD implementation above? Or is it possible to merge the implementations of the BSDs (only the mib differ as far as I can see).

Possibly could even check for path_len <= 1 for additional safety. There are a few very subtle differences between the FreeBSD & DragonFly block and what I implemented, but I imagine they could be merged if thought best. While they are nearly identical, I have this purist view that as there is no shared lineage in the kernel-side implementation of these sysctls, they may as well stay seperate here.

kennytm · 2017-11-29T18:03:42Z

This line is too long, causing CI to fail :(
[00:04:06] tidy error: /checkout/src/libstd/sys/unix/os.rs:246: line longer than 100 chars

I'll wrap that line.

jakllsch · 2017-11-29T22:06:34Z

CI might have missed this:

error[E0277]: the trait bound core::result::Result<path::PathBuf, io::error::Error>: core::ops::FnOnce<(io::error::Error,)> is not satisfied
--> /local/jakllsch/rust/rust/src/libstd/sys/unix/os.rs:253:14
|
253 | sysctl().or_else(procfs())
| ^^^^^^^ the trait core::ops::FnOnce<(io::error::Error,)> is not implemented for core::result::Result<path::PathBuf, io::error::Error>

kennytm · 2017-11-29T22:24:15Z

The CI for PR only runs on Linux, so it will definitely miss any change made to NetBSD.

kennytm · 2017-11-30T15:07:08Z

sysctl().or_else(procfs) without the last ().

I had tried that and got:
error[E0593]: function is expected to take 1 argument, but it takes 0 arguments --> /local/jakllsch/rust/rust/src/libstd/sys/unix/os.rs:253:14 | 253 | sysctl().or_else(procfs) | ^^^^^^^ expected function that takes 1 argument error: aborting due to previous error
I'd really like to keep the prototypes/signatures of sysctl() and procfs() identical..

@jakllsch Oh ok. Call through a closure sysctl().or_else(|_| procfs()) then.

kennytm · 2017-11-30T18:03:01Z

@jakllsch Are we supposed to wait for you to squash the commits or can these be merged as-is?

Use the CTL_KERN.KERN_PROC_ARGS.-1.KERN_PROC_PATHNAME sysctl in preference over the /proc/curproc/exe symlink. Additionally, perform more validation of aformentioned symlink. Particularly on pre-8.x NetBSD this symlink will point to '/' when accurate information is unavailable.

jakllsch · 2017-11-30T21:32:50Z

@kennytm I've squashed it.

kennytm · 2017-11-30T21:42:44Z

Thanks for the contribution!

@bors r+

bors · 2017-11-30T21:42:45Z

📌 Commit ccef969 has been approved by kennytm

… r=kennytm NetBSD: add sysctl backend for std::env::current_exe Use the CTL_KERN.KERN_PROC_ARGS.-1.KERN_PROC_PATHNAME sysctl in preference over the /proc/curproc/exe symlink. Additionally, perform more validation of aformentioned symlink. Particularly on pre-8.x NetBSD this symlink will point to '/' when accurate information is unavailable.

Rollup of 13 pull requests - Successful merges: #45880, #46280, #46373, #46376, #46385, #46386, #46387, #46392, #46400, #46401, #46405, #46412, #46421 - Failed merges:

rust-highfive assigned kennytm Nov 29, 2017

kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 29, 2017

kennytm reviewed Nov 29, 2017
View reviewed changes

kennytm requested changes Nov 30, 2017
View reviewed changes

kennytm approved these changes Nov 30, 2017
View reviewed changes

jakllsch force-pushed the netbsd-kern_proc_pathname branch from 8de0cde to ccef969Compare November 30, 2017 18:18

kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 30, 2017

kennytm mentioned this pull request Dec 1, 2017
Rollup of 13 pull requests #46430
Merged

bors added a commit that referenced this pull request Dec 1, 2017
Auto merge of#46430- kennytm:rollup, r=kennytm
7051754
Rollup of 13 pull requests - Successful merges: #45880, #46280, #46373, #46376, #46385, #46386, #46387, #46392, #46400, #46401, #46405, #46412, #46421 - Failed merges:

bors merged commit ccef969 into rust-lang:masterDec 2, 2017

Uh oh!

Uh oh!

Conversation

jakllsch commented Nov 29, 2017

Uh oh!

rust-highfive commented Nov 29, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakllsch commented Nov 29, 2017

Uh oh!

kennytm commented Nov 29, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kennytm commented Nov 30, 2017

Uh oh!

jakllsch commented Nov 30, 2017

Uh oh!

kennytm commented Nov 30, 2017

Uh oh!

bors commented Nov 30, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants