Add utility functions for invoking a subprocess #491
Conversation
|
src/process.c
Outdated
|
|
||
| int exec_ret = execvp(cmd, argv.data); | ||
|
|
||
| RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING("Failed to execute process: %d", exec_ret); |
There was a problem hiding this comment.
| RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING("Failed to execute process: %d", exec_ret); | |
| if (exec_ret == -1) { | |
| int error = errno; | |
| RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING("Failed to execute process: %d (%s)", error, strerror(error)); | |
| } |
There was a problem hiding this comment.
execvp will only return if there is an error. The docs say it will return -1, but I'm not really sure what we'd do in this process if it returned anything else anyway - we'd still want to let the user know that something went wrong. I'll take the strerror change though.
There was a problem hiding this comment.
Applied the error string portion in 7d97d42.
src/process.c
Outdated
| for(size_t i = 0; i < argv.size; i++) { | ||
| argv.data[i] = NULL; | ||
| } |
There was a problem hiding this comment.
i do not think we need to do this, because rcutils_string_array_fini does that with deallocation.
There was a problem hiding this comment.
I'm actually "borrowing" the individual strings from the passed argument array, and if they're still referenced in argv when I deallocate it, they will be deallocated as well.
This really shouldn't be an rcutils_string_array_t anyway. I think I'll switch to just a calloc/memcpy to simplify the code and make that clearer.
There was a problem hiding this comment.
Changed to a standalone zero_allocate in 7d97d42.
| &allocator, "allocator is invalid", return ); | ||
|
|
||
| #if defined _WIN32 || defined __CYGWIN__ | ||
| CloseHandle(process->handle); |
There was a problem hiding this comment.
just curious here, Linux doesn't require to close the process ?
There was a problem hiding this comment.
Linux doesn't actually open any handles to the process, as far as I can tell. The PID of the child process will remain allocated until the parent process reads the exit code from that process once, at which point the PID is released for reuse elsewhere.
This is why I note in the docs:
Upon successful invocation of this function, subsequent calls will produce undefined behavior. It should typically be followed by a call to rcutils_process_close().
This new function and associated functions provide a cross-platform mechanism for invoking a command as a subprocess of the currently running process. Signed-off-by: Scott K Logan <logans@cottsay.net>
cottsay
force-pushed
the
cottsay/subprocess
branch
from
b0b8737 to
7d97d42
Compare
|
Alright, I rebased to take the string join change from |
Signed-off-by: Scott K Logan <logans@cottsay.net>
cottsay
force-pushed
the
cottsay/subprocess
branch
from
13872ec to
5f893a3
Compare
|
Fresh CI:
|
This new function and associated functions provide a cross-platform mechanism for invoking a command as a subprocess of the currently running process.
There is some nuanced behavior for how process IDs are freed on Linux and Windows that influenced this API:
Requires #490