Almost finish the env module
#141
Conversation
|
☔ The latest upstream changes (presumably #110) made this pull request unmergeable. Please resolve the merge conflicts. |
Only function missing is `home_dir`
|
@homunkulus try |
Almost finish the `env` module The only function missing is `home_dir`
|
Hm. Travis doesn't appear here, but this shows as passed on Travis's website. |
|
☀️ Test successful - status-travis |
src/libc/mod.rs
Outdated
| pub value: Cow<'static, [u8]>, | ||
| } | ||
|
|
||
| #[allow(unused)] |
There was a problem hiding this comment.
Hmm, why is the allow(unused) required? ENV_PTR is used in _init down below, no? (Same with AUXVAL)
| if input.is_empty() { | ||
| return None; | ||
| } | ||
| let pos = memchr(b'=', &input[1..]).map(|p| p + 1); |
There was a problem hiding this comment.
It wasn't obvious to me why input is sliced like this. Could you copy this comment from std?
// Strategy (copied from glibc): Variable name and value are separated
// by an ASCII equals sign '='. Since a variable name must not be
// empty, allow variable names starting with an equals sign. Skip all
// malformed lines.
I'm not sure if it's around already but if it is then all these 1.. slicing operations should point out to that comment.
src/sys/linux/os.rs
Outdated
| use vec; | ||
|
|
||
| static ENV_LOCK: () = (); | ||
| // TODO(steed): Synchronize environment access once we have mutexes. |
There was a problem hiding this comment.
Could you open an issue about this? Now that we have threads this one may go very wrong if two or more threads try to set_env the same variable :-/.
| &*ENV | ||
| } | ||
|
|
||
| pub unsafe fn environ() -> *const *const c_char { |
There was a problem hiding this comment.
Hmm, does this function actually make it to the std public API? It any case, I think this should panic like libc::errno. We should probably document these differences from std since there now would be two of them.
There was a problem hiding this comment.
No, it doesn't make it to the public API, this is the stub for spawning new commands right now.
There was a problem hiding this comment.
Hmm, does that means that spawned processes don't inherit their parent's environment? We should create an issue about that if that's the case.
|
@homunkulus delegate+ (I think the review comment is not picked by Homu) |
|
✌️ @tbu- can now approve this pull request |
|
Since Addressed the other comments. @homunkulus r+ |
|
📌 Commit 3c2bd06 has been approved by |
Almost finish the `env` module The only function missing is `home_dir`
|
☀️ Test successful - status-travis |
The only function missing is
home_dir