exec functions #76
exec functions #76
Conversation
The |
|
All the variadic functions that have been implemented so far did it without the use of the va_list crate, so I just followed along. |
include/bits/exec.h
Outdated
| argv[i] = va_arg(ap, char *); | ||
| argv[i] = NULL; | ||
| va_end(ap); | ||
| return _execl(path, argv); |
There was a problem hiding this comment.
Note: musl calls execv here.
include/bits/exec.h
Outdated
| } | ||
| } | ||
|
|
||
| int execlp(const char* file, const char* argv0, ...) { |
There was a problem hiding this comment.
Check out the implementation in musl. You shouldn't assume that a box has /bin/sh here. This is more like an implementation of system.
There was a problem hiding this comment.
I have no idea what's going on in the musl implementation of execvp
include/bits/exec.h
Outdated
| } | ||
| } | ||
|
|
||
| int execvp(const char* file, char *const _argv[]) { |
| while !(*env).is_null() { | ||
| let slice = c_str(*env); | ||
| // Should always contain a =, but worth checking | ||
| if let Some(sep) = slice.iter().position(|&c| c == b'=') { |
There was a problem hiding this comment.
You will also probably want to make sure that the value isn't just an = or one side isn't empty.
There was a problem hiding this comment.
This is true, empty env values are expected
src/unistd/src/lib.rs
Outdated
| // unimplemented!(); | ||
| // } | ||
| #[no_mangle] | ||
| pub extern "C" fn _execl(path: *const c_char, args: *const *mut c_char) -> c_int { |
There was a problem hiding this comment.
If you use execv like musl. You can remove this function. It has the same signature.
src/unistd/src/lib.rs
Outdated
| } | ||
|
|
||
| #[no_mangle] | ||
| pub extern "C" fn _execle( |
There was a problem hiding this comment.
Same signature as execve. Just use execve in the C implementation of execle.
include/bits/exec.h
Outdated
| argv[i] = va_arg(ap, char *); | ||
| envp = va_arg(ap, char **); | ||
| va_end(ap); | ||
| return _execle(path, argv, envp); |
src/unistd/src/lib.rs
Outdated
| } | ||
| /* | ||
| *#[no_mangle] | ||
| *pub extern "C" fn execvp(file: *const c_char, argv: *const *mut c_char) -> c_int { |
There was a problem hiding this comment.
Just remove this if you're going to comment it out.
There was a problem hiding this comment.
I was hoping someone could have a look at it and see a way to fix it.
|
I'm leaving execvp and execlp for someone smarter. |
dlrobertson
force-pushed
the
unistd
branch
2 times, most recently
from
afee3d3
to
06efe08
Compare
src/platform/src/lib.rs
Outdated
| @@ -54,6 +61,10 @@ pub unsafe fn c_str_n(s: *const c_char, n: usize) -> &'static [u8] { | |||
| slice::from_raw_parts(s as *const u8, size as usize) | |||
| } | |||
|
|
|||
| pub unsafe fn cstr_from_bytes_with_nul_unchecked(bytes: &[u8]) -> *const c_char { | |||
| &*(bytes as *const [u8] as *const c_char) | |||
There was a problem hiding this comment.
Shouldn't this be bytes.as_ptr() as *const c_char?
But this function seems to be unused currently anyway.
There was a problem hiding this comment.
@ids1024 I took this straight from CString.
There was a problem hiding this comment.
The &* is only needed in CStr because that function returns a reference, not a pointer.
And it looks like the use of a pointer to a slice is because of the non-optimal way CStr is defined:
pub struct CStr {
// FIXME: this should not be represented with a DST slice but rather with
// just a raw `c_char` along with some form of marker to make
// this an unsized type. Essentially `sizeof(&CStr)` should be the
// same as `sizeof(&c_char)` but `CStr` should be an unsized type.
inner: [c_char]
}Using the cast as *const [u8] as *const c_char may work, but I think that's potentially undefined behavior. as_ptr() exists for this purpose.
Implement execve, execl, and execv and implement setuid and setgid. Co-authored-by: Dan Robertson <[email protected]>
src/platform/src/lib.rs
Outdated
| @@ -62,7 +62,7 @@ pub unsafe fn c_str_n(s: *const c_char, n: usize) -> &'static [u8] { | |||
| } | |||
|
|
|||
| pub unsafe fn cstr_from_bytes_with_nul_unchecked(bytes: &[u8]) -> *const c_char { | |||
| &*(bytes as *const [u8] as *const c_char) | |||
| bytes.as_ptr() as *const c_char) | |||
There was a problem hiding this comment.
You accidentally left in the close-paren there.
This isn't really ready for merge yet but I want to open this up for discussion. In issue #64, it was mentioned that we don't want C functions in our header files, but we need some intermediary C to interact with va_lists and char**s. I tried to implement execvp without header C, as you will see commented out, but couldn't get it working. I'm open to suggestions.