Skip to content

Commit 4afebf1

Browse files
refactor: popen and pclose error reporting
* Set errno for popen and pclose for WIN32 just like POSIX does (at least mingw64+wine seems to decode the error message without doing anything extra) * A work around for popen/pclose: complicated but works * Refactored popen/pclose into custom implementation and added c++ wrappers * Removed exceptions and cleaned up process pipe api * Refactored popen/pclose into C++ class * Fixed trivial issues for Linux * Simplified the move semantics by avoiding virtual functions * Moved error reporting into common_pipe * Simplified open and close in case the pipe is already opened/closed * Fixed error handling for MSVC, which very weird: MSVC rejects strerror, suggest to use "secure" strerror_s, but then does not supply strerrorlen_s GCC does not provide strerror_s, looks like strerror is good enough there. * MSVC is not following any standards
1 parent e4ce497 commit 4afebf1

File tree

5 files changed

+354
-144
lines changed

5 files changed

+354
-144
lines changed

source/matplot/backend/gnuplot.cpp

+21-26
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@
44

55
#include "gnuplot.h"
66

7-
#include <cstdlib>
8-
#include <iostream>
97
#include <matplot/util/common.h>
108
#include <matplot/util/popen.h>
9+
#include <iostream>
1110
#include <regex>
1211
#include <thread>
12+
#include <cstring>
13+
#include <cstdlib>
1314

1415
#ifdef __has_include
1516
#if __has_include(<filesystem>)
@@ -70,17 +71,18 @@ namespace matplot::backend {
7071
}
7172

7273
// Open the gnuplot pipe_
74+
int perr;
7375
if constexpr (windows_should_persist_by_default) {
74-
pipe_ = POPEN("gnuplot --persist", "w");
76+
perr = pipe_.open("gnuplot --persist");
7577
} else {
76-
pipe_ = POPEN("gnuplot", "w");
78+
perr = pipe_.open("gnuplot");
7779
}
7880

7981
// Check if everything is OK
80-
if (!pipe_) {
81-
std::cerr << "Opening the gnuplot pipe_ failed!" << std::endl;
82-
std::cerr
83-
<< "Please install gnuplot 5.2.6+: http://www.gnuplot.info"
82+
if (perr != 0 || !pipe_.opened()) {
83+
std::cerr << "Opening the gnuplot failed: ";
84+
std::cerr << pipe_.error() << std::endl;
85+
std::cerr << "Please install gnuplot 5.2.6+: http://www.gnuplot.info"
8486
<< std::endl;
8587
}
8688
}
@@ -97,9 +99,6 @@ namespace matplot::backend {
9799
flush_commands();
98100
run_command("exit");
99101
flush_commands();
100-
if (pipe_) {
101-
PCLOSE(pipe_);
102-
}
103102
}
104103

105104
bool gnuplot::is_interactive() { return output_.empty(); }
@@ -281,8 +280,7 @@ namespace matplot::backend {
281280
if constexpr (dont_let_it_close_too_fast) {
282281
last_flush_ = std::chrono::high_resolution_clock::now();
283282
}
284-
fputs("\n", pipe_);
285-
fflush(pipe_);
283+
pipe_.flush("\n");
286284
if constexpr (trace_commands) {
287285
std::cout << "\n\n\n\n" << std::endl;
288286
}
@@ -294,19 +292,19 @@ namespace matplot::backend {
294292
}
295293

296294
void gnuplot::run_command(const std::string &command) {
297-
if (!pipe_) {
295+
if (!pipe_.opened()) {
298296
return;
299297
}
300-
size_t pipe_capacity = gnuplot_pipe_capacity(pipe_);
298+
size_t pipe_capacity = gnuplot_pipe_capacity(pipe_.file());
301299
if (command.size() + bytes_in_pipe_ > pipe_capacity) {
302300
flush_commands();
303301
bytes_in_pipe_ = 0;
304302
}
305303
if (!command.empty()) {
306-
fputs(command.c_str(), pipe_);
304+
pipe_.write(command);
307305
}
308-
// fputs("; ", pipe_);
309-
fputs("\n", pipe_);
306+
// proc_write(&pipe_, "; ");
307+
pipe_.write("\n");
310308
bytes_in_pipe_ += command.size();
311309
if constexpr (trace_commands) {
312310
std::cout << command << std::endl;
@@ -323,11 +321,9 @@ namespace matplot::backend {
323321
static std::string terminal_type;
324322
const bool dont_know_term_type = terminal_type.empty();
325323
if (dont_know_term_type) {
326-
terminal_type =
327-
run_and_get_output("gnuplot -e \"show terminal\" 2>&1");
328-
terminal_type = std::regex_replace(
329-
terminal_type, std::regex("[^]*terminal type is ([^ ]+)[^]*"),
330-
"$1");
324+
terminal_type = run_and_get_output("gnuplot -e \"show terminal\" 2>&1");
325+
terminal_type = std::regex_replace(terminal_type,
326+
std::regex("[^]*terminal type is ([^ ]+)[^]*"), "$1");
331327
const bool still_dont_know_term_type = terminal_type.empty();
332328
if (still_dont_know_term_type) {
333329
terminal_type = "qt";
@@ -337,9 +333,8 @@ namespace matplot::backend {
337333
}
338334

339335
bool gnuplot::terminal_is_available(std::string_view term) {
340-
std::string msg =
341-
run_and_get_output("gnuplot -e \"set terminal " +
342-
std::string(term.data()) + "\" 2>&1");
336+
std::string msg = run_and_get_output("gnuplot -e \"set terminal " +
337+
std::string(term.data()) + "\" 2>&1");
343338
return msg.empty();
344339
}
345340

source/matplot/backend/gnuplot.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <matplot/detail/config.h>
99
#include <matplot/backend/backend_interface.h>
10+
#include <matplot/util/popen.h>
1011
#include <array>
1112
#include <chrono>
1213
#include <tuple>
@@ -115,8 +116,8 @@ namespace matplot::backend {
115116
}
116117

117118
private:
118-
// Pipe to gnuplot process
119-
FILE *pipe_;
119+
// Process pipe to gnuplot
120+
opipe pipe_;
120121

121122
// How many bytes we put in the pipe
122123
size_t bytes_in_pipe_{0};

source/matplot/util/common.cpp

+3-19
Original file line numberDiff line numberDiff line change
@@ -50,26 +50,10 @@ namespace matplot {
5050
iequals(str, "no");
5151
}
5252

53-
struct pipe_deleter {
54-
int operator()(FILE* pipe) const {
55-
if (int status = PCLOSE(pipe); status != -1)
56-
return status;
57-
throw std::system_error{errno, std::system_category(), "pclose"};
58-
}
59-
};
60-
6153
std::string run_and_get_output(const std::string &cmd) {
62-
std::unique_ptr<FILE, pipe_deleter> pipe(POPEN(cmd.c_str(), "r"));
63-
if (!pipe) {
64-
throw std::system_error{errno, std::system_category(), cmd};
65-
}
66-
std::array<char, 128> buffer{};
67-
std::string result;
68-
while (fgets(buffer.data(), static_cast<int>(buffer.size()),
69-
pipe.get()) != nullptr) {
70-
result += buffer.data();
71-
}
72-
return result;
54+
auto res = std::string{};
55+
shell_read(cmd, res);
56+
return res;
7357
}
7458

7559
std::string escape(std::string_view label) {

0 commit comments

Comments
 (0)