Skip to content

Commit fb90149

Browse files
committed
Refactored popen/pclose into C++ class
1 parent 1ef2019 commit fb90149

File tree

4 files changed

+165
-148
lines changed

4 files changed

+165
-148
lines changed

source/matplot/backend/gnuplot.cpp

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,13 @@ namespace matplot::backend {
7272
// Open the gnuplot pipe_
7373
int perr;
7474
if constexpr (windows_should_persist_by_default) {
75-
perr = pipe_open(&pipe_, "gnuplot --persist", "w");
75+
perr = pipe_.open("gnuplot --persist");
7676
} else {
77-
perr = pipe_open(&pipe_, "gnuplot", "w");
77+
perr = pipe_.open("gnuplot");
7878
}
7979

8080
// Check if everything is OK
81-
if (perr != 0 || !pipe_is_valid(pipe_)) {
81+
if (perr != 0 || !pipe_.opened()) {
8282
std::cerr << "Opening the gnuplot pipe_ failed: "
8383
<< std::strerror(perr) << std::endl;
8484
std::cerr << "Please install gnuplot 5.2.6+: http://www.gnuplot.info"
@@ -98,9 +98,6 @@ namespace matplot::backend {
9898
flush_commands();
9999
run_command("exit");
100100
flush_commands();
101-
if (pipe_is_valid(pipe_)) {
102-
pipe_close(&pipe_);
103-
}
104101
}
105102

106103
bool gnuplot::is_interactive() { return output_.empty(); }
@@ -282,7 +279,7 @@ namespace matplot::backend {
282279
if constexpr (dont_let_it_close_too_fast) {
283280
last_flush_ = std::chrono::high_resolution_clock::now();
284281
}
285-
pipe_flush(&pipe_, "\n");
282+
pipe_.flush("\n");
286283
if constexpr (trace_commands) {
287284
std::cout << "\n\n\n\n" << std::endl;
288285
}
@@ -294,19 +291,19 @@ namespace matplot::backend {
294291
}
295292

296293
void gnuplot::run_command(const std::string &command) {
297-
if (!pipe_is_valid(pipe_)) {
294+
if (!pipe_.opened()) {
298295
return;
299296
}
300-
size_t pipe_capacity = gnuplot_pipe_capacity(pipe_.file);
297+
size_t pipe_capacity = gnuplot_pipe_capacity(pipe_.file());
301298
if (command.size() + bytes_in_pipe_ > pipe_capacity) {
302299
flush_commands();
303300
bytes_in_pipe_ = 0;
304301
}
305302
if (!command.empty()) {
306-
pipe_write(&pipe_, command);
303+
pipe_.write(command);
307304
}
308305
// proc_write(&pipe_, "; ");
309-
pipe_write(&pipe_, "\n");
306+
pipe_.write("\n");
310307
bytes_in_pipe_ += command.size();
311308
if constexpr (trace_commands) {
312309
std::cout << command << std::endl;

source/matplot/backend/gnuplot.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ namespace matplot::backend {
117117

118118
private:
119119
// Process pipe to gnuplot
120-
ProcPipe pipe_;
120+
opipe pipe_;
121121

122122
// How many bytes we put in the pipe
123123
size_t bytes_in_pipe_{0};

source/matplot/util/popen.cpp

Lines changed: 99 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
#include <matplot/util/popen.h>
22
#include <array>
3+
#include <system_error>
34

45
#ifdef _WIN32
56

67
#include <io.h>
78

8-
int pipe_open(ProcPipe *pipe, const char *cmd, const char *mode)
9+
int common_pipe::open(const std::string& cmd, char mode)
910
{
10-
if (cmd == nullptr || mode == nullptr || (mode[0] != 'r' && mode[0] != 'w') ||
11-
pipe == nullptr)
12-
return EINVAL;
11+
if (opened())
12+
close(); // prevent resource leak
1313
HANDLE hChildStdinRd, hChildStdinWr, hStdin, hStdout;
1414
SECURITY_ATTRIBUTES saAttr{};
1515
// Set up security attributes for inheritable handles
@@ -19,7 +19,7 @@ int pipe_open(ProcPipe *pipe, const char *cmd, const char *mode)
1919

2020
// Create a pipe for the child process's input
2121
if (!CreatePipe(&hChildStdinRd, &hChildStdinWr, &saAttr, 0))
22-
return GetLastError();
22+
return error(GetLastError(), "CreatePipe");
2323

2424
// Ensure the write handle to the pipe is not inherited by child
2525
// processes
@@ -58,7 +58,7 @@ int pipe_open(ProcPipe *pipe, const char *cmd, const char *mode)
5858
PROCESS_INFORMATION pi;
5959

6060
// Create the child process, while hiding the window
61-
if (!CreateProcess(NULL, const_cast<char *>(cmd), NULL, NULL, TRUE,
61+
if (!CreateProcess(NULL, const_cast<char *>(cmd.c_str()), NULL, NULL, TRUE,
6262
CREATE_NO_WINDOW, NULL, NULL, &si, &pi)) {
6363
auto err = GetLastError();
6464
CloseHandle(hChildStdinRd);
@@ -67,17 +67,20 @@ int pipe_open(ProcPipe *pipe, const char *cmd, const char *mode)
6767
CloseHandle(hStdout);
6868
return err;
6969
}
70-
pipe->hProcess = pi.hProcess;
71-
pipe->hThread = pi.hThread;
70+
hProcess = pi.hProcess;
71+
hThread = pi.hThread;
7272

7373
// Close unnecessary handles
7474
CloseHandle(hChildStdinRd);
7575
CloseHandle(hStdout);
7676

7777
// Create a FILE pointer from the write handle to the child process's
7878
// input
79-
pipe->file = _fdopen(_open_osfhandle((intptr_t)hChildStdinWr, 0), mode);
80-
if (pipe->file == nullptr) {
79+
if (mode == 'r')
80+
file_ = _fdopen(_open_osfhandle((intptr_t)hChildStdinWr, 0), "r");
81+
else
82+
file_ = _fdopen(_open_osfhandle((intptr_t)hChildStdinWr, 0), "w");
83+
if (file_ == nullptr) {
8184
auto err = GetLastError();
8285
CloseHandle(hChildStdinWr);
8386
CloseHandle(hStdin);
@@ -88,32 +91,30 @@ int pipe_open(ProcPipe *pipe, const char *cmd, const char *mode)
8891
return 0;
8992
}
9093

91-
int pipe_close(ProcPipe *pipe, int *exit_code)
94+
int common_pipe::close(int *exit_code)
9295
{
93-
// The following does not work for GetExitCodeProcess:
94-
// HANDLE hFile = (HANDLE)_get_osfhandle(_fileno(file));
95-
if (!pipe_is_valid(pipe))
96-
return EINVAL;
96+
if (!opened())
97+
return error(EINVAL, "common_pipe::close");
9798
// Close the pipe to process:
98-
fclose(pipe->file);
99-
pipe->file = nullptr;
99+
fclose(file_);
100+
file_ = nullptr;
100101
// Wait for the process to finish
101-
if (auto r = WaitForSingleObject(pipe->hProcess, INFINITE); r != WAIT_OBJECT_0) {
102-
CloseHandle(pipe->hThread);
103-
CloseHandle(pipe->hProcess);
102+
if (auto r = WaitForSingleObject(hProcess, INFINITE); r != WAIT_OBJECT_0) {
103+
CloseHandle(hThread);
104+
CloseHandle(hProcess);
104105
return ECHILD;
105106
}
106107
// Retrieve the exit code
107108
if (exit_code != nullptr) {
108-
if (BOOL r = GetExitCodeProcess(pipe->hProcess, (LPDWORD)exit_code); !r) {
109+
if (BOOL r = GetExitCodeProcess(hProcess, (LPDWORD)exit_code); !r) {
109110
auto err = GetLastError();
110-
CloseHandle(pipe->hThread);
111-
CloseHandle(pipe->hProcess);
111+
CloseHandle(hThread);
112+
CloseHandle(hProcess);
112113
return err;
113114
}
114115
}
115-
CloseHandle(pipe->hThread);
116-
CloseHandle(pipe->hProcess);
116+
CloseHandle(hThread);
117+
CloseHandle(hProcess);
117118
return 0;
118119
}
119120
#endif // _WIN32 implementtion
@@ -123,116 +124,128 @@ int pipe_close(ProcPipe *pipe, int *exit_code)
123124
#include <cerrno>
124125
#include <sys/wait.h> // waitpid
125126

126-
int pipe_open(ProcPipe *p, const char *cmd, const char *mode)
127+
int common_pipe::open(const std::string &cmd, char mode)
127128
{
128129
constexpr auto READ = 0u;
129130
constexpr auto WRITE = 1u;
130-
if (cmd == nullptr || mode == nullptr || (mode[0] != 'r' && mode[0] != 'w') || p == nullptr)
131-
return EINVAL;
132-
133131
int fd[2];
134132
if (pipe(fd) == -1)
135-
return errno;
136-
if ((p->pid = fork()) == -1)
137-
return errno;
133+
return error(errno, "pipe");
134+
if ((pid = fork()) == -1)
135+
return error(errno, "fork");
138136

139-
if (p->pid == 0) { // child process
140-
if (mode[0] == 'r') {
137+
if (pid == 0) { // child process
138+
if (mode == 'r') {
141139
close(fd[READ]); // Close the READ end of the pipe
142140
dup2(fd[WRITE], 1); // Redirect stdout to pipe
143-
} else { // (mode[0] == 'w')
141+
} else { // (mode == 'w')
144142
close(fd[WRITE]); // Close the WRITE end of the pipe
145143
dup2(fd[READ], 0); // Redirect stdin to pipe
146144
}
147-
setpgid(p->pid, p->pid); // Needed so negative PIDs can kill children of /bin/sh
148-
execl("/bin/sh", "/bin/sh", "-c", cmd, nullptr); // returns only upon errors
149-
return errno;
145+
setpgid(pid, pid); // Needed so negative PIDs can kill children of /bin/sh
146+
execl("/bin/sh", "/bin/sh", "-c", cmd.c_str(), nullptr);
147+
// execl returns only upon error
148+
std::exit(EXIT_FAILURE);
150149
} else {
151-
if (mode[0] == 'r') {
150+
if (mode == 'r') {
152151
close(fd[WRITE]); // Close the WRITE end of the pipe since parent's
153152
// fd is read-only
154-
} else { // (mode[0] == 'w')
153+
} else { // (mode == 'w')
155154
close(fd[READ]); // Close the READ end of the pipe since parent's fd
156155
// is write-only
157156
}
158157
}
159-
p->file = (mode[0] == 'r') ? fdopen(fd[READ], "r") : fdopen(fd[WRITE], "w");
158+
if (mode == 'r')
159+
file_ = fdopen(fd[READ], "r");
160+
else
161+
file_ = fdopen(fd[WRITE], "w");
162+
if (file_ == nullptr)
163+
return error(errno, "fdopen");
160164
return 0;
161165
}
162166

163167
/// Closes the pipe opened by popen2 and waits for termination
164-
int pipe_close(ProcPipe *pipe, int *exit_code)
168+
int common_pipe::close(int *exit_code)
165169
{
166-
if (!proc_is_good(pipe))
167-
return EINVAL;
168-
fclose(pipe->file);
169-
while (waitpid(pipe->pid, exit_code, 0) == -1) {
170+
if (!valid())
171+
return 0;
172+
fclose(file_);
173+
file_ = nullptr;
174+
while (waitpid(pid, exit_code, 0) == -1) {
170175
if (errno != EINTR) {
171-
*exit_code = -1;
176+
if (exit_code != nullptr)
177+
*exit_code = -1;
172178
break;
173179
}
174180
}
175-
pipe->file = nullptr;
176181
return 0;
177182
}
178183

179184
#endif // POSIX implementation
180185

181-
int pipe_write(ProcPipe *p, std::string_view data)
186+
inline int common_pipe::error(int code, const std::string& what) const
187+
{
188+
if (exceptions_)
189+
throw std::system_error{code, std::generic_category(), what};
190+
return code;
191+
}
192+
193+
int opipe::write(std::string_view data)
182194
{
183195
constexpr auto CSIZE = sizeof(std::string_view::value_type);
184-
if (!pipe_is_valid(p))
185-
return EINVAL;
186-
if (auto sz = std::fwrite(data.data(), CSIZE, data.length(), p->file);
196+
if (!opened())
197+
return error(EINVAL, "opipe::write");
198+
if (auto sz = std::fwrite(data.data(), CSIZE, data.length(), file_);
187199
sz != data.size()) {
188-
if (auto err = std::ferror(p->file); err != 0)
189-
return EIO;
190-
if (auto err = std::feof(p->file); err != 0)
191-
return EIO;
200+
if (auto err = std::ferror(file_); err != 0)
201+
return error(EIO, "fwrite error");
202+
if (auto err = std::feof(file_); err != 0)
203+
return error(EIO, "fwrite eof");
192204
}
193-
if (auto res = std::fflush(p->file); res != 0)
194-
return errno;
195205
return 0;
196206
}
197207

198-
int pipe_flush(ProcPipe *p, std::string_view data)
208+
int opipe::flush(std::string_view data)
199209
{
200-
if (!pipe_is_valid(p))
201-
return EINVAL;
210+
if (!opened())
211+
return error(EINVAL, "opipe::flush");
202212
if (!data.empty())
203-
if (auto err = pipe_write(p, data); err != 0)
204-
return err;
205-
if (auto res = std::fflush(p->file); res != 0)
206-
return errno;
213+
if (auto err = write(data); err != 0)
214+
return error(err, "opipe::write");
215+
if (auto res = std::fflush(file_); res != 0)
216+
return error(errno, "fflush");
207217
return 0;
208218
}
209219

210-
int shell_write(const std::string &cmd, std::string_view data)
220+
int ipipe::read(std::string &data)
211221
{
212-
auto pipe = ProcPipe{};
213-
if (auto err = pipe_open(&pipe, cmd.c_str(), "w"); err != 0)
214-
return err;
215-
if (!data.empty())
216-
if (auto err = pipe_flush(&pipe, data); err != 0)
217-
return err;
218-
if (auto err = pipe_close(&pipe); err != 0)
219-
return err;
220-
return 0;
221-
}
222-
223-
int shell_read(const std::string &cmd, std::string &data)
224-
{
225-
auto pipe = ProcPipe{};
226-
if (auto err = pipe_open(&pipe, cmd.c_str(), "r"); err != 0)
227-
return err;
222+
if (!opened())
223+
return error(EINVAL, "ipipe::read");
228224
data.clear();
229225
auto buffer = std::array<char, 128>{};
230-
while (!std::feof(pipe.file) && !std::ferror(pipe.file)) {
231-
auto count = std::fread(buffer.data(), sizeof(char), buffer.size(), pipe.file);
226+
while (!std::feof(file_) && !std::ferror(file_)) {
227+
auto count =
228+
std::fread(buffer.data(), sizeof(char), buffer.size(), file_);
232229
if (count > 0)
233230
data.append(buffer.data(), count);
234231
}
235-
if (auto err = pipe_close(&pipe); err != 0)
236-
return err;
232+
if (std::ferror(file_))
233+
return error(EIO, "fread");
237234
return 0;
238235
}
236+
237+
int shell_write(const std::string &cmd, std::string &data)
238+
{
239+
auto pipe = opipe{};
240+
if (auto err = pipe.open(cmd); err != 0)
241+
return err;
242+
return pipe.write(data);
243+
}
244+
245+
int shell_read(const std::string &cmd, std::string &data)
246+
{
247+
auto pipe = ipipe{};
248+
if (auto err = pipe.open(cmd); err != 0)
249+
return err;
250+
return pipe.read(data);
251+
}

0 commit comments

Comments
 (0)