Re: [boost] Formal Review of Proposed Boost.Process library

Subject: Re: [boost] Formal Review of Proposed Boost.Process library
From: Max Sobolev (macsmr_at_[hidden])
Date: 2011-02-19 10:55:49

(First, please, excuse me for my "english".)

This variant of the Boost.Process library should NOT (--never--) be 
accepted.

The Boost.Process must be implemented as a DSEL (probably through the 
Boost.Proto expression templates framework) with a nice (commonly known) 
syntax like:

using boost::process;
using process::arg;
namespace fs = boost::filesystem;

process ls("ls"), grep("grep");
fs::path libs = "/usr/lib";

auto pipe = ls [--arg("reverse") % -arg('l') % libs]  |  grep ["^d"];

run(pipe);
//   or:
// pipe();
//   or:
// ls();

This approach provide a declarative, not an imperative programming style. (A 
lot of work done by expression templates' inner mechanics.)

The library must support:

* i/o redirection
* process piping; [implicit pipes]
* named/unnamed pipe objects; [explicit pipes]
* runned processes surfing (boost::process::processes_iterator):
  - CreateToolhelp32Snapshot()/Process32First()/Process32Next() API 
functions in Windows
  - /proc filesystem parsing in linux/some unix
* loaded shared libraries surfing (boost::process::modules_iterator):
  - CreateToolhelp32Snapshot()/Module32First()/Module32Next() API functions 
in Windows
  - /proc filesystem parsing in linux/some unix
* child processes surfing (boost::process::children_iterator)
  - parsing processes_iterator's range results
  - /proc filesystem parsing in linux/(perhaps) some unix
  - empty iterator range by default
* runned thread per process surfing (boost::process::threads_iterator)
  - filtered results of 
CreateToolhelp32Snapshot()/Thread32First()/Thread32Next() API functions in 
Windows
  - /proc filesystem parsing in linux (see /proc/[pid]/task filesystem 
branch since linux 2.6.0-test6)
  - empty iterator range by default
* runned thread surfing (boost::threads_iterator)
  - CreateToolhelp32Snapshot()/Thread32First()/Thread32Next() API functions 
in Windows
  - FOR-EACH process IN system:
      back_inserter(boost::process::threads_iterator(process),
                    boost::process::threads_iterator(),
                    threads);
* (any) process stats
* process creation
* daemonization
  - through Service API on Windows
* process security and privacy aspects (probably this is subject for an 
another separate library, part of which must be integrated with the 
Boost.Process)

_________________________________
More examples:

  using boost::process;
  using process::arg;
  using process::env;
  namespace fs = boost::filesystem;

  // echo $PATH   # (%PATH% in Windows)
  process("echo") [env("PATH")]
    ();
  // or: run(process("echo") [env("PATH")]);

  // ps -aux > ps.out
  process ps("ps");
  run(ps [-arg('a', 'u', 'x')] > "ps.out");

  process::_this_ > "file.out" < "file.in"; // probably static_assert() on 
Windows platform
  process::_this_ >> "file.out" < "file.in";
  std::cin >> ...; // read from "file.in"
  std::cout << ...; // write to "file.out"

  // grep -i -n searched_word file.in > file.out:
  process grep("grep");
  run(
    grep [-arg('i') % -arg('n') % "searched_word" % "file.in"] > 
"file.out");

  // cat /etc/rc.d/xinetd | grep -v '^#' | sed '/^$/d' > file.out
  process cat("cat"), grep("grep"), sed("sed");
  fs::path conf("/etc/rc.d/xinetd");
  auto pipe = cat [conf]  |  grep [-arg('v') % "^#"]  |  sed ["/^$/d"] > 
"file.out";
  pipe();

  using namespace process::placeholders;
  process rm("rm");
  rm [arg("filename") % "non_existent"] > "file.out", _2 >& _1;
  rm(); // or: run(rm);

_________________________________

Some mini-review (concerning interface design only) on the proposed variant 
of the library:

* process::find_executable_in_path() is a redundant function.
His job must be done by default (when the filename() path's suffix supplied 
only instead of the full/relative path), if it's supposed on the target 
platform when the command shell gets the prog name from the user. In rare 
case the user can manipulates paths through the env(ironment variables) 
class (or put the full path explicitly), if default behavior is not 
suitable.
By the way, a parameter and an return value must have been the 
boost::filesystem::basic_path<>, not a std::string. Obviously, the library 
must be integrated with the Boost.Filesystem.

* A "stream behavior" boost::function<> mechanism is not obvious and not 
straightforward; unwarrantly hard to use.

* environment(-variables) and args-list are solid abstractions (with a sharp 
behavior), not data structures.

* child process subtype is a wrong abstraction (his "child" property isn't 
enough for his existence). Name it process simply. I.e. delete a child 
subtype from an inheritance tree; In Unix environment (similar in Windows) 
all processes, except INIT, have a parent: each process is a child almost 
without exceptions.

* ----code excerpt from the User Guide--
  int exit_code = child.wait();
#if defined(BOOST_POSIX_API)
  if (WIFEXITED(exit_code))
    exit_code = WEXITSTATUS(exit_code);
#endif
  std::cout << exit_code << std::endl;
----------------------------------------

IFDEFs must be incapsulated in the wait() member function. Signal 
terminating/stopping status of process can be abstracted into another terms, 
which are clear enough for a non "signal-conformant" platforms.