Exit child process when execvp fails - #4577
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
| execvp(executable, argv.get()); | ||
| // execvp returns only if execution failed. | ||
| perror(executable); | ||
| _exit(EXIT_FAILURE); |
There was a problem hiding this comment.
The indentation here must be fixed. And why _exit() instead of exit()?
There was a problem hiding this comment.
Indentation fixed. _exit() is deliberate: after a failed exec the child still holds copies of the parent's stdio buffers and atexit handlers, so exit() could flush buffered output a second time and run the parent's cleanup in the child. _exit() terminates without touching either, which is the usual pattern on the failed-exec path.
There was a problem hiding this comment.
Pull request overview
This PR improves robustness in the ScrollView utility layer by ensuring forked child processes don’t accidentally continue execution when execvp() fails, and by making socket flushing safer when send() is interrupted or fails.
Changes:
- In
SVSync::StartProcess(), handleexecvp()failure by reporting the error and exiting the child immediately. - In
SVNetwork::Flush(), handlesend()failures safely (avoid erasing with a negative count) and retry onEINTR(non-Windows).
| #ifndef _WIN32 | ||
| if (errno == EINTR) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Added to the standard includes rather than relying on transitive inclusion.
| int i = | ||
| send(stream_, msg_buffer_out_.c_str(), msg_buffer_out_.length(), 0); | ||
|
|
There was a problem hiding this comment.
Switched to auto, which picks up ssize_t on POSIX and int on Winsock, so the return value is no longer narrowed.
Summary
Handle the failure case of
execvp()inSVSync::StartProcess().If
execvp()fails, the child process now reports the error and exits immediately instead of continuing along the parent's execution path.