exec functions #76

sajattack · 2018-03-13T00:32:03Z

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.

ids1024 · 2018-03-14T05:30:10Z

we need some intermediary C to interact with va_lists and char**s.

char** certainly is possible to deal with in Rust, though it can be awkward. There is however, not currently a standard way to implement C variadic functions (it looks like they do plan to add support for that, though).

The va_list crate which relibc already depends on seems to make it possible to implement variadic functions (looking at the example in their documentation).

sajattack · 2018-03-14T19:41:55Z

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.

dlrobertson

Failing fmt check.
nit: in your C for loops
- spaces around = and <
- could you add braces to the for loop?

dlrobertson · 2018-03-15T23:04:45Z

include/bits/exec.h

+ argv[i] = va_arg(ap, char *);
+ argv[i] = NULL;
+ va_end(ap);
+ return _execl(path, argv);


Note: musl calls execv here.

dlrobertson · 2018-03-15T23:08:35Z

include/bits/exec.h

+ }
+}
+
+int execlp(const char* file, const char* argv0, ...) {


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.

I have no idea what's going on in the musl implementation of execvp

dlrobertson · 2018-03-15T23:09:38Z

include/bits/exec.h

+ }
+}
+
+int execvp(const char* file, char *const _argv[]) {


Same as above.

dlrobertson · 2018-03-15T23:22:41Z

src/platform/src/redox/mod.rs

+ 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'=') {


You will also probably want to make sure that the value isn't just an = or one side isn't empty.

This is true, empty env values are expected

dlrobertson · 2018-03-15T23:26:31Z

src/unistd/src/lib.rs

-// unimplemented!();
-// }
+#[no_mangle]
+pub extern "C" fn _execl(path: *const c_char, args: *const *mut c_char) -> c_int {


If you use execv like musl. You can remove this function. It has the same signature.

dlrobertson · 2018-03-15T23:28:18Z

src/unistd/src/lib.rs

+}
+
+#[no_mangle]
+pub extern "C" fn _execle(


Same signature as execve. Just use execve in the C implementation of execle.

dlrobertson · 2018-03-15T23:28:32Z

include/bits/exec.h

+ argv[i] = va_arg(ap, char *);
+ envp = va_arg(ap, char **);
+ va_end(ap);
+ return _execle(path, argv, envp);


Use execve here.

dlrobertson · 2018-03-15T23:33:09Z

src/unistd/src/lib.rs

-}
+/*
+ *#[no_mangle]
+ *pub extern "C" fn execvp(file: *const c_char, argv: *const *mut c_char) -> c_int {


Just remove this if you're going to comment it out.

I was hoping someone could have a look at it and see a way to fix it.

sajattack · 2018-03-16T03:06:08Z

I'm leaving execvp and execlp for someone smarter.

ids1024 · 2018-04-07T05:17:09Z

src/platform/src/lib.rs

@@ -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)


Shouldn't this be bytes.as_ptr() as *const c_char?

But this function seems to be unused currently anyway.

@ids1024 I took this straight from CString.

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]>

ids1024 · 2018-04-09T03:52:36Z

src/platform/src/lib.rs

@@ -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)


You accidentally left in the close-paren there.

dlrobertson suggested changes Mar 15, 2018

View reviewed changes

dlrobertson force-pushed the unistd branch 2 times, most recently from afee3d3 to 06efe08 Compare March 22, 2018 16:17

ids1024 reviewed Apr 7, 2018

View reviewed changes

Implement basic exec functions

a128a4c

Implement execve, execl, and execv and implement setuid and setgid. Co-authored-by: Dan Robertson <[email protected]>

dlrobertson force-pushed the unistd branch from 06efe08 to a128a4c Compare April 8, 2018 13:56

Update lib.rs

74d477f

ids1024 reviewed Apr 9, 2018

View reviewed changes

Remove paren

15c57fc

sajattack closed this Feb 16, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

exec functions #76

exec functions #76

sajattack commented Mar 13, 2018 •

edited

Loading

ids1024 commented Mar 14, 2018

sajattack commented Mar 14, 2018

dlrobertson left a comment

dlrobertson Mar 15, 2018

dlrobertson Mar 15, 2018

sajattack Mar 16, 2018

dlrobertson Mar 15, 2018

dlrobertson Mar 15, 2018

jackpot51 Mar 22, 2018

dlrobertson Mar 15, 2018

dlrobertson Mar 15, 2018

dlrobertson Mar 15, 2018

dlrobertson Mar 15, 2018

sajattack Mar 15, 2018

sajattack commented Mar 16, 2018

ids1024 Apr 7, 2018

sajattack Apr 7, 2018

ids1024 Apr 7, 2018

ids1024 Apr 9, 2018

exec functions #76

exec functions #76

Conversation

sajattack commented Mar 13, 2018 • edited Loading

ids1024 commented Mar 14, 2018

sajattack commented Mar 14, 2018

dlrobertson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sajattack commented Mar 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sajattack commented Mar 13, 2018 •

edited

Loading