aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2017-12-14 14:24:38 +0200
committerKaren Arutyunov <karen@codesynthesis.com>2017-12-15 13:38:33 +0300
commit53b4f58c78e21cbc442891c2ce2a2b99a32e47bc (patch)
treef2b892650367a44332d7a169ede8aa9e60e6a3c8
parentceb8f4abba2cfc7ac51385fa59693c641151c8d2 (diff)
Add process::pipe struct, extend process API
-rw-r--r--libbutl/curl.cxx40
-rw-r--r--libbutl/curl.mxx8
-rw-r--r--libbutl/openssl.cxx38
-rw-r--r--libbutl/openssl.mxx12
-rw-r--r--libbutl/process-run.cxx6
-rw-r--r--libbutl/process-run.txx104
-rw-r--r--libbutl/process.cxx65
-rw-r--r--libbutl/process.ixx24
-rw-r--r--libbutl/process.mxx39
-rw-r--r--libbutl/sendmail.ixx2
-rw-r--r--tests/process-run/driver.cxx7
11 files changed, 265 insertions, 80 deletions
diff --git a/libbutl/curl.cxx b/libbutl/curl.cxx
index 5ff81ab..ee61d6d 100644
--- a/libbutl/curl.cxx
+++ b/libbutl/curl.cxx
@@ -8,6 +8,8 @@
// C includes.
+#include <cassert>
+
#ifndef __cpp_lib_modules
#include <string>
@@ -40,7 +42,7 @@ using namespace std;
namespace butl
{
- int curl::
+ process::pipe curl::
map_in (nullfd_t, method_proto mp, io_data& d)
{
switch (mp)
@@ -53,16 +55,18 @@ namespace butl
case http_get:
{
d.pipe.in = fdnull (); // /dev/null
- return d.pipe.in.get ();
+ return pipe (d.pipe);
}
}
- return -1;
+ assert (false); // Can't be here.
+ return pipe ();
}
- int curl::
+ process::pipe curl::
map_in (const path& f, method_proto mp, io_data& d)
{
+ pipe r;
switch (mp)
{
case ftp_put:
@@ -84,12 +88,17 @@ namespace butl
if (f.string () == "-")
{
d.pipe = fdopen_pipe (fdopen_mode::binary);
+ r = pipe (d.pipe);
+
out.open (move (d.pipe.out));
}
else
+ {
d.pipe.in = fdnull (); // /dev/null
+ r = pipe (d.pipe);
+ }
- return d.pipe.in.get ();
+ return r;
}
case ftp_get:
case http_get:
@@ -98,10 +107,11 @@ namespace butl
}
}
- return -1;
+ assert (false); // Can't be here.
+ return r;
}
- int curl::
+ process::pipe curl::
map_out (nullfd_t, method_proto mp, io_data& d)
{
switch (mp)
@@ -113,16 +123,18 @@ namespace butl
case http_post: // May or may not produce output.
{
d.pipe.out = fdnull ();
- return d.pipe.out.get (); // /dev/null
+ return pipe (d.pipe); // /dev/null
}
}
- return -1;
+ assert (false); // Can't be here.
+ return pipe ();
}
- int curl::
+ process::pipe curl::
map_out (const path& f, method_proto mp, io_data& d)
{
+ pipe r;
switch (mp)
{
case ftp_get:
@@ -134,6 +146,8 @@ namespace butl
// Note: no need for any options, curl writes to stdout by default.
//
d.pipe = fdopen_pipe (fdopen_mode::binary);
+ r = pipe (d.pipe);
+
in.open (move (d.pipe.in));
}
else
@@ -141,9 +155,10 @@ namespace butl
d.options.push_back ("-o");
d.options.push_back (f.string ().c_str ());
d.pipe.out = fdnull (); // /dev/null
+ r = pipe (d.pipe);
}
- return d.pipe.out.get ();
+ return r;
}
case ftp_put:
{
@@ -151,7 +166,8 @@ namespace butl
}
}
- return -1;
+ assert (false); // Can't be here.
+ return r;
}
curl::method_proto curl::
diff --git a/libbutl/curl.mxx b/libbutl/curl.mxx
index 57d6e56..7beb844 100644
--- a/libbutl/curl.mxx
+++ b/libbutl/curl.mxx
@@ -169,20 +169,20 @@ LIBBUTL_MODEXPORT namespace butl
std::string storage;
};
- int
+ pipe
map_in (nullfd_t, method_proto, io_data&);
- int
+ pipe
map_in (const path&, method_proto, io_data&);
template <typename I>
typename std::enable_if<is_other<I>::value, I>::type
map_in (I&&, method_proto, io_data&);
- int
+ pipe
map_out (nullfd_t, method_proto, io_data&);
- int
+ pipe
map_out (const path&, method_proto, io_data&);
template <typename O>
diff --git a/libbutl/openssl.cxx b/libbutl/openssl.cxx
index 89cf8ed..48b0c1d 100644
--- a/libbutl/openssl.cxx
+++ b/libbutl/openssl.cxx
@@ -36,21 +36,24 @@ using namespace std;
namespace butl
{
- int openssl::
+ process::pipe openssl::
map_in (nullfd_t, io_data& d)
{
d.pipe.in = fdnull (); // /dev/null
- return d.pipe.in.get ();
+ return pipe (d.pipe);
}
- int openssl::
+ process::pipe openssl::
map_in (const path& f, io_data& d)
{
+ pipe r;
if (f.string () == "-")
{
// Note: no need for any options, openssl reads from stdin by default.
//
d.pipe = fdopen_pipe (fdopen_mode::binary);
+ r = pipe (d.pipe);
+
out.open (move (d.pipe.out));
}
else
@@ -58,12 +61,13 @@ namespace butl
d.options.push_back ("-in");
d.options.push_back (f.string ().c_str ());
d.pipe.in = fdnull (); // /dev/null
+ r = pipe (d.pipe);
}
- return d.pipe.in.get ();
+ return r;
}
- int openssl::
+ process::pipe openssl::
map_in (fdstream_mode m, io_data& d)
{
assert (m == fdstream_mode::text || m == fdstream_mode::binary);
@@ -73,26 +77,30 @@ namespace butl
d.pipe = fdopen_pipe (m == fdstream_mode::binary
? fdopen_mode::binary
: fdopen_mode::none);
-
+ pipe r (d.pipe);
+
out.open (move (d.pipe.out));
- return d.pipe.in.get ();
+ return r;
}
- int openssl::
+ process::pipe openssl::
map_out (nullfd_t, io_data& d)
{
d.pipe.out = fdnull ();
- return d.pipe.out.get (); // /dev/null
+ return pipe (d.pipe); // /dev/null
}
- int openssl::
+ process::pipe openssl::
map_out (const path& f, io_data& d)
{
+ pipe r;
if (f.string () == "-")
{
// Note: no need for any options, openssl writes to stdout by default.
//
d.pipe = fdopen_pipe (fdopen_mode::binary);
+ r = pipe (d.pipe);
+
in.open (move (d.pipe.in), fdstream_mode::skip);
}
else
@@ -100,12 +108,13 @@ namespace butl
d.options.push_back ("-out");
d.options.push_back (f.string ().c_str ());
d.pipe.out = fdnull (); // /dev/null
+ r = pipe (d.pipe);
}
- return d.pipe.out.get ();
+ return r;
}
- int openssl::
+ process::pipe openssl::
map_out (fdstream_mode m, io_data& d)
{
assert (m == fdstream_mode::text || m == fdstream_mode::binary);
@@ -115,8 +124,9 @@ namespace butl
d.pipe = fdopen_pipe (m == fdstream_mode::binary
? fdopen_mode::binary
: fdopen_mode::none);
-
+ pipe r (d.pipe);
+
in.open (move (d.pipe.in), fdstream_mode::skip);
- return d.pipe.out.get ();
+ return r;
}
}
diff --git a/libbutl/openssl.mxx b/libbutl/openssl.mxx
index 42afc5e..a465aad 100644
--- a/libbutl/openssl.mxx
+++ b/libbutl/openssl.mxx
@@ -152,26 +152,26 @@ LIBBUTL_MODEXPORT namespace butl
small_vector<const char*, 2> options;
};
- int
+ pipe
map_in (nullfd_t, io_data&);
- int
+ pipe
map_in (const path&, io_data&);
- int
+ pipe
map_in (fdstream_mode, io_data&);
template <typename I>
typename std::enable_if<is_other<I>::value, I>::type
map_in (I&&, io_data&);
- int
+ pipe
map_out (nullfd_t, io_data&);
- int
+ pipe
map_out (const path&, io_data&);
- int
+ pipe
map_out (fdstream_mode, io_data&);
template <typename O>
diff --git a/libbutl/process-run.cxx b/libbutl/process-run.cxx
index fd8abe3..446eec0 100644
--- a/libbutl/process-run.cxx
+++ b/libbutl/process-run.cxx
@@ -41,9 +41,9 @@ namespace butl
const process_path& pp,
const char* cmd[],
const char* const* envvars,
- int in,
- int out,
- int err)
+ process::pipe in,
+ process::pipe out,
+ process::pipe err)
{
try
{
diff --git a/libbutl/process-run.txx b/libbutl/process-run.txx
index d8d4cb7..9e4ccfc 100644
--- a/libbutl/process-run.txx
+++ b/libbutl/process-run.txx
@@ -20,27 +20,101 @@ LIBBUTL_MODEXPORT namespace butl //@@ MOD Clang needs this for some reason.
}
}
- inline int process_stdin (int v) {assert (v >= 0); return v;}
- inline int process_stdout (int v) {assert (v >= 0); return v;}
- inline int process_stderr (int v) {assert (v >= 0); return v;}
+ inline process::pipe
+ process_stdin (int v)
+ {
+ assert (v >= 0);
+ return process::pipe (v, -1);
+ }
+
+ inline process::pipe
+ process_stdout (int v)
+ {
+ assert (v >= 0);
+ return process::pipe (-1, v);
+ }
+
+ inline process::pipe
+ process_stderr (int v)
+ {
+ assert (v >= 0);
+ return process::pipe (-1, v);
+ }
+
+ inline process::pipe
+ process_stdin (const auto_fd& v)
+ {
+ assert (v.get () >= 0);
+ return process::pipe (v.get (), -1);
+ }
+
+ inline process::pipe
+ process_stdout (const auto_fd& v)
+ {
+ assert (v.get () >= 0);
+ return process::pipe (-1, v.get ());
+ }
+
+ inline process::pipe
+ process_stderr (const auto_fd& v)
+ {
+ assert (v.get () >= 0);
+ return process::pipe (-1, v.get ());
+ }
+
+ inline process::pipe
+ process_stdin (const fdpipe& v)
+ {
+ assert (v.in.get () >= 0 && v.out.get () >= 0);
+ return process::pipe (v);
+ }
+
+ inline process::pipe
+ process_stdout (const fdpipe& v)
+ {
+ assert (v.in.get () >= 0 && v.out.get () >= 0);
+ return process::pipe (v);
+ }
+
+ inline process::pipe
+ process_stderr (const fdpipe& v)
+ {
+ assert (v.in.get () >= 0 && v.out.get () >= 0);
+ return process::pipe (v);
+ }
- inline int
- process_stdin (const auto_fd& v) {assert (v.get () >= 0); return v.get ();}
+ // Not necessarily a real pipe, so only a single end is constrained to be a
+ // valid file descriptor.
+ //
+ inline process::pipe
+ process_stdin (const process::pipe& v)
+ {
+ assert (v.in >= 0);
+ return v;
+ }
- inline int
- process_stdout (const auto_fd& v) {assert (v.get () >= 0); return v.get ();}
+ inline process::pipe
+ process_stdout (const process::pipe& v)
+ {
+ assert (v.out >= 0);
+ return v;
+ }
- inline int
- process_stderr (const auto_fd& v) {assert (v.get () >= 0); return v.get ();}
+ inline process::pipe
+ process_stderr (const process::pipe& v)
+ {
+ assert (v.out >= 0);
+ return v;
+ }
LIBBUTL_SYMEXPORT process
process_start (const dir_path* cwd,
const process_path& pp,
const char* cmd[],
const char* const* envvars,
- int in,
- int out,
- int err);
+ process::pipe in,
+ process::pipe out,
+ process::pipe err);
template <typename V, typename T>
inline const char*
@@ -68,9 +142,9 @@ LIBBUTL_MODEXPORT namespace butl //@@ MOD Clang needs this for some reason.
// Map stdin/stdout/stderr arguments to their integer values, as expected
// by the process constructor.
//
- int in_i (process_stdin (std::forward<I> (in)));
- int out_i (process_stdout (std::forward<O> (out)));
- int err_i (process_stderr (std::forward<E> (err)));
+ process::pipe in_i (process_stdin (std::forward<I> (in)));
+ process::pipe out_i (process_stdout (std::forward<O> (out)));
+ process::pipe err_i (process_stderr (std::forward<E> (err)));
// Construct the command line array.
//
diff --git a/libbutl/process.cxx b/libbutl/process.cxx
index b9f3b2f..4a6d5f5 100644
--- a/libbutl/process.cxx
+++ b/libbutl/process.cxx
@@ -170,17 +170,6 @@ namespace butl
} while (*p != nullptr);
}
- process::
- process (const process_path& pp, const char* args[],
- process& in, int out, int err,
- const char* cwd,
- const char* const* envvars)
- : process (pp, args, in.in_ofd.get (), out, err, cwd, envvars)
- {
- assert (in.in_ofd.get () != -1); // Should be a pipe.
- in.in_ofd.reset (); // Close it on our side.
- }
-
#ifndef _WIN32
static process_path
@@ -303,10 +292,14 @@ namespace butl
process::
process (const process_path& pp, const char* args[],
- int in, int out, int err,
+ pipe pin, pipe pout, pipe perr,
const char* cwd,
const char* const* envvars)
{
+ int in (pin.in);
+ int out (pout.out);
+ int err (perr.out);
+
fdpipe out_fd;
fdpipe in_ofd;
fdpipe in_efd;
@@ -1013,10 +1006,14 @@ namespace butl
process::
process (const process_path& pp, const char* args[],
- int in, int out, int err,
+ pipe pin, pipe pout, pipe perr,
const char* cwd,
const char* const* envvars)
{
+ int in (pin.in);
+ int out (pout.out);
+ int err (perr.out);
+
auto fail = [] (const char* m = nullptr)
{
throw process_error (m == nullptr ? last_error_msg () : m);
@@ -1487,6 +1484,31 @@ namespace butl
// data presence at the reading end of any of these pipes means that
// DLLs initialization successfully passed.
//
+ // If the process output stream is redirected to a pipe, check that
+ // we have access to its reading end to probe it for data presence.
+ //
+ // @@ Unfortunately we can't distinguish a pipe that erroneously
+ // missing the reading end from the pipelined program output
+ // stream.
+#if 0
+ auto bad_pipe = [&get_osfhandle] (const pipe& p) -> bool
+ {
+ if (p.in != -1 || // Pipe provided by the user.
+ p.out == -1 || // Pipe created by ctor.
+ p.out == -2 || // Null device.
+ fdterm (p.out)) // Terminal.
+ return false;
+
+ // No reading end, so make sure that the file descriptor is a pipe.
+ //
+ return GetFileType (get_osfhandle (p.out, false)) ==
+ FILE_TYPE_PIPE;
+ };
+
+ if (bad_pipe (pout) || bad_pipe (perr))
+ assert (false);
+#endif
+
system_clock::time_point st (system_clock::now ());
// Unlock the mutex to let other processes to be spawned while we are
@@ -1496,14 +1518,22 @@ namespace butl
il.unlock ();
l.unlock ();
- auto probe_fd = [&get_osfhandle] (int fd) -> bool
+ // Probe for data presence the reading end of the pipe that is
+ // connected to the process standard output/error stream. Note that
+ // the pipe can be created by us or provided by the user.
+ //
+ auto probe_fd = [&get_osfhandle] (int fd, int ufd) -> bool
{
- if (fd == -1)
+ // Can't be both created by us and provided by the user.
+ //
+ assert (fd == -1 || ufd == -1);
+
+ if (fd == -1 && ufd == -1)
return false;
char c;
DWORD n;
- HANDLE h (get_osfhandle (fd, false));
+ HANDLE h (get_osfhandle (fd != -1 ? fd : ufd, false));
return PeekNamedPipe (h, &c, 1, &n, nullptr, nullptr) && n == 1;
};
@@ -1519,7 +1549,8 @@ namespace butl
twd += wd;
if (r != WAIT_TIMEOUT ||
- probe_fd (in_ofd.in.get ()) || probe_fd (in_efd.in.get ()))
+ probe_fd (in_ofd.in.get (), pout.in) ||
+ probe_fd (in_efd.in.get (), perr.in))
break;
}
diff --git a/libbutl/process.ixx b/libbutl/process.ixx
index fb076cf..a0e2de6 100644
--- a/libbutl/process.ixx
+++ b/libbutl/process.ixx
@@ -137,6 +137,19 @@ namespace butl
}
inline process::
+ process (const process_path& pp, const char* args[],
+ int in, int out, int err,
+ const char* cwd,
+ const char* const* envvars)
+ : process (pp,
+ args,
+ pipe (in, -1), pipe (-1, out), pipe (-1, err),
+ cwd,
+ envvars)
+ {
+ }
+
+ inline process::
process (const char* args[],
int in, int out, int err,
const char* cwd,
@@ -144,6 +157,17 @@ namespace butl
: process (path_search (args[0]), args, in, out, err, cwd, envvars) {}
inline process::
+ process (const process_path& pp, const char* args[],
+ process& in, int out, int err,
+ const char* cwd,
+ const char* const* envvars)
+ : process (pp, args, in.in_ofd.get (), out, err, cwd, envvars)
+ {
+ assert (in.in_ofd.get () != -1); // Should be a pipe.
+ in.in_ofd.reset (); // Close it on our side.
+ }
+
+ inline process::
process (const char* args[],
process& in, int out, int err,
const char* cwd,
diff --git a/libbutl/process.mxx b/libbutl/process.mxx
index abf7d35..af6f995 100644
--- a/libbutl/process.mxx
+++ b/libbutl/process.mxx
@@ -255,12 +255,35 @@ LIBBUTL_MODEXPORT namespace butl
// temporarily change args[0] (see path_search() for details).
//
process (const char* [],
- int = 0, int = 1, int = 2,
+ int in = 0, int out = 1, int err = 2,
const char* cwd = nullptr,
const char* const* envvars = nullptr);
process (const process_path&, const char* [],
- int = 0, int = 1, int = 2,
+ int in = 0, int out = 1, int err = 2,
+ const char* cwd = nullptr,
+ const char* const* envvars = nullptr);
+
+ // If the descriptors are pipes that you have created, then you should use
+ // this constructor instead to communicate this information.
+ //
+ // For generality, if the "other" end of the pipe is -1, then assume this
+ // is not a pipe.
+ //
+ struct pipe
+ {
+ int in = -1;
+ int out = -1;
+
+ pipe () = default;
+ pipe (int i, int o): in (i), out (o) {}
+
+ explicit
+ pipe (const fdpipe& p): in (p.in.get ()), out (p.out.get ()) {}
+ };
+
+ process (const process_path&, const char* [],
+ pipe in, pipe out, pipe err,
const char* cwd = nullptr,
const char* const* envvars = nullptr);
@@ -273,12 +296,12 @@ LIBBUTL_MODEXPORT namespace butl
// lhs.wait ();
//
process (const char* [],
- process&, int = 1, int = 2,
+ process&, int out = 1, int err = 2,
const char* cwd = nullptr,
const char* const* envvars = nullptr);
process (const process_path&, const char* [],
- process&, int = 1, int = 2,
+ process&, int out = 1, int err = 2,
const char* cwd = nullptr,
const char* const* envvars = nullptr);
@@ -427,10 +450,10 @@ LIBBUTL_MODEXPORT namespace butl
// exit status.
//
// The I/O/E arguments determine the child's stdin/stdout/stderr. They can
- // be of type int, auto_fd (and, in the future, perhaps also fd_pipe,
- // string, buffer, etc). For example, the following call will make stdin
- // read from /dev/null, stdout redirect to stderr, and inherit the parent's
- // stderr.
+ // be of type int, auto_fd, fd_pipe and process::pipe (and, in the future,
+ // perhaps also string, buffer, etc). For example, the following call will
+ // make stdin read from /dev/null, stdout redirect to stderr, and inherit
+ // the parent's stderr.
//
// process_run (fdnull (), 2, 2, ...)
//
diff --git a/libbutl/sendmail.ixx b/libbutl/sendmail.ixx
index 942b9a8..4fb3dac 100644
--- a/libbutl/sendmail.ixx
+++ b/libbutl/sendmail.ixx
@@ -60,7 +60,7 @@ LIBBUTL_MODEXPORT namespace butl //@@ MOD Clang needs this for some reason.
process& p (*this);
p = process_start_callback (cmdc,
- pipe.in,
+ pipe,
2, // No output expected so redirect to stderr.
std::forward<E> (err),
"sendmail",
diff --git a/tests/process-run/driver.cxx b/tests/process-run/driver.cxx
index 765bd7d..1c41659 100644
--- a/tests/process-run/driver.cxx
+++ b/tests/process-run/driver.cxx
@@ -99,6 +99,13 @@ main (int argc, const char* argv[])
assert (run (fdnull (), 2, 2, p, "-c", "-o", "abc"));
assert (run (fdnull (), 1, 1, p, "-c", "-e", "abc"));
+ {
+ fdpipe pipe (fdopen_pipe ());
+ process pr (process_start (pipe, process::pipe (-1, 1), 2, p, "-c", "-i"));
+ pipe.close ();
+ assert (pr.wait ());
+ }
+
// Argument conversion.
//
assert (run (0, 1, 2, p, "-c", "-o", "abc"));