Skip to content

Commit

Permalink
Merge #30081: refactor: Remove unused code from subprocess.h header
Browse files Browse the repository at this point in the history
5a11d30 refactor, subprocess: Remove unused stream API calls (Hennadii Stepanov)
05b6f87 refactor, subprocess: Remove unused `Popen::child_created_` data member (Hennadii Stepanov)
9e1ccf5 refactor, subprocess: Remove unused `Popen::poll()` (Hennadii Stepanov)
24b53fc refactor, subprocess: Remove `Popen::pid()` (Hennadii Stepanov)

Pull request description:

  This PR continues #29961.

  Please note that `Popen::poll()` is not required for #29868 anymore.

ACKs for top commit:
  theuni:
    Easy code review ACK 5a11d30 since it's all removals :)
  theStack:
    Code-review ACK 5a11d30

Tree-SHA512: 11f984f8384c337782d058afa80011e88087a1b5a3ed6e4678d492e6c232b706a26312463c5dd8c529aa477497c8afca9f54574e0e36e3edd5675bd5d8424bbb
  • Loading branch information
fanquake committed May 11, 2024
2 parents 4d3f1d0 + 5a11d30 commit b47c393
Showing 1 changed file with 1 addition and 75 deletions.
76 changes: 1 addition & 75 deletions src/util/subprocess.h
Expand Up @@ -902,17 +902,9 @@ class Streams
* Command provided in a single string.
* wait() - Wait for the child to exit.
* retcode() - The return code of the exited child.
* pid() - PID of the spawned child.
* poll() - Check the status of the running child.
* send(...) - Send input to the input channel of the child.
* communicate(...) - Get the output/error from the child and close the channels
* from the parent side.
* input() - Get the input channel/File pointer. Can be used for
* customizing the way of sending input to child.
* output() - Get the output channel/File pointer. Usually used
in case of redirection. See piping examples.
* error() - Get the error channel/File pointer. Usually used
in case of redirection.
*/
class Popen
{
Expand Down Expand Up @@ -956,14 +948,10 @@ class Popen
execute_process();
}

int pid() const noexcept { return child_pid_; }

int retcode() const noexcept { return retcode_; }

int wait() noexcept(false);

int poll() noexcept(false);

void set_out_buf_cap(size_t cap) { stream_.set_out_buf_cap(cap); }

void set_err_buf_cap(size_t cap) { stream_.set_err_buf_cap(cap); }
Expand Down Expand Up @@ -1001,15 +989,6 @@ class Popen
return communicate(nullptr, 0);
}

FILE* input() { return stream_.input(); }
FILE* output() { return stream_.output();}
FILE* error() { return stream_.error(); }

/// Stream close APIs
void close_input() { stream_.input_.reset(); }
void close_output() { stream_.output_.reset(); }
void close_error() { stream_.error_.reset(); }

private:
template <typename F, typename... Args>
void init_args(F&& farg, Args&&... args);
Expand All @@ -1033,7 +1012,6 @@ class Popen
std::vector<std::string> vargs_;
std::vector<char*> cargv_;

bool child_created_ = false;
// Pid of the child process
int child_pid_ = -1;

Expand Down Expand Up @@ -1068,7 +1046,7 @@ inline int Popen::wait() noexcept(false)
return 0;
#else
int ret, status;
std::tie(ret, status) = util::wait_for_child_exit(pid());
std::tie(ret, status) = util::wait_for_child_exit(child_pid_);
if (ret == -1) {
if (errno != ECHILD) throw OSError("waitpid failed", errno);
return 0;
Expand All @@ -1081,56 +1059,6 @@ inline int Popen::wait() noexcept(false)
#endif
}

inline int Popen::poll() noexcept(false)
{
#ifdef __USING_WINDOWS__
int ret = WaitForSingleObject(process_handle_, 0);
if (ret != WAIT_OBJECT_0) return -1;

DWORD dretcode_;
if (FALSE == GetExitCodeProcess(process_handle_, &dretcode_))
throw OSError("GetExitCodeProcess", 0);

retcode_ = (int)dretcode_;
CloseHandle(process_handle_);

return retcode_;
#else
if (!child_created_) return -1; // TODO: ??

int status;

// Returns zero if child is still running
int ret = waitpid(child_pid_, &status, WNOHANG);
if (ret == 0) return -1;

if (ret == child_pid_) {
if (WIFSIGNALED(status)) {
retcode_ = WTERMSIG(status);
} else if (WIFEXITED(status)) {
retcode_ = WEXITSTATUS(status);
} else {
retcode_ = 255;
}
return retcode_;
}

if (ret == -1) {
// From subprocess.py
// This happens if SIGCHLD is set to be ignored
// or waiting for child process has otherwise been disabled
// for our process. This child is dead, we cannot get the
// status.
if (errno == ECHILD) retcode_ = 0;
else throw OSError("waitpid failed", errno);
} else {
retcode_ = ret;
}

return retcode_;
#endif
}

inline void Popen::execute_process() noexcept(false)
{
#ifdef __USING_WINDOWS__
Expand Down Expand Up @@ -1233,8 +1161,6 @@ inline void Popen::execute_process() noexcept(false)
throw OSError("fork failed", errno);
}

child_created_ = true;

if (child_pid_ == 0)
{
// Close descriptors belonging to parent
Expand Down

0 comments on commit b47c393

Please sign in to comment.