Boost logo

Boost :

Subject: Re: [boost] [gsoc] Boost.Process done
From: Boris Schaeling (boris_at_[hidden])
Date: 2010-09-10 18:21:48


On Fri, 10 Sep 2010 01:33:53 +0200, Ilya Sokolov <ilyasokol_at_[hidden]>
wrote:

> [...]The following should solve problems that I know of:
>
> typedef std::vector<std::pair<fileno, handle::native_type>>
> child_streams;
>
> typedef boost::function2<
> std::pair<handle, handle>,
> fileno,
> const child_streams&
> > behavior;
>
> std::pair<handle, handle> inherit(fileno fn, const child_streams&)
> {
> #ifdef _WIN32
> handle child_end(GetStdHandle(DWORD(-10-fn)), dont_close);
> #else
> handle child_end(fn, dont_close);
> #endif
> handle parent_end;
> return std::make_pair(child_end, parent_end);
> }
>
> class redirect_to
> {
> fileno to_;
> public:
> redirect_to(fileno to): to_(to) {}
> std::pair<handle, handle>
> operator()(fileno fn, const child_streams& cs)
> {
> BOOST_ASSERT(fn > to_);
> child_streams::const_iterator it =
> // find pair in cs where it->first == to_
> #ifndef _WIN32
> handle child_end(posix_dup2(it->second, fn));
> #else
> // ...
> #endif
> handle parent_end;
> return std::make_pair(child_end, parent_end);
> }
> };
>enum std_fileno
> {
> stdin, stdout, stderr
> };
>
> Example of usage:
>
> ctx.stderr = redirect_to(stdout);

I'm a bit confused: If you use fileno like above on Windows (0=stdin,
1=stdout and 2=stderr) then we could also use an enumeration? At least
your implementation of inherit makes me think so that it's not the file
descriptor itself you are interested in but only the information whether
the standard input, output or error stream is configured. Then there would
be no difference between your and mine solution?

If you are interested in the actual value of fileno though (after all the
POSIX implementation of inherit uses fileno directly) I see a couple of
problems.

Passing fileno to a stream behavior's operator() is actually the opposite
of what we expect a stream behavior to do: We want it to create the
child's streams for us. For example bp::behavior::pipe should return the
child's end of the pipe. That said it would be strange to pass a fileno to
bp::behavior::pipe if we actually expect bp::behavior::pipe to return a
fileno to us.

A more serious problem is that on POSIX a child process can inherit
streams with filenos greater than 2 (eg. see
<http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/file_descriptors_setup.cpp>).
If you use bp::behavior::pipe like in the example what fileno should we
pass to operator()? The pipe needs to know whether it is used to configure
an input or output stream.

As on Windows handles are used to represent standard streams what should
the first parameter passed to operator() look like? We know that the
fileno of the standard error stream of the child process will be 2 on
POSIX. But what will the handle look like on Windows? How will redirect_to
know it should redirect the standard error stream? Well, fortunately
redirect_to doesn't need to access the child's standard error stream on
Windows but can simply dup and return the to_ stream. :-) But what kind of
information would a handle passed to operator() provide?

Thinking it through I'm more convinced that we need to pass the
information to a stream behavior whether it's used to configure the
standard input, output or error stream. As filenos greater than 2 can be
inherited on POSIX a bit field might be better as it would allow us to
distinguish input and output streams (a filenos greater than 2 are either
input or output streams but no standard streams).

> [...]
>> As I also think the default constructor of context shouldn't throw:
>> What if we add another optional parameter to bp::behavior::inherit? A
>> bool variable could be used to indicate whether bp::behavior::inherit
>> should throw if dup()/DuplicateHandle() fails.
>
> No, it shouldn't call dup()/DuplicateHandle() in the first place.

Convinced - the dont_close flag sounds like a good idea! I'll update the
code on the weekend.

Boris


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk