Boost logo

Boost :

Subject: Re: [boost] [gsoc] Boost.Process done
From: Ilya Sokolov (ilyasokol_at_[hidden])
Date: 2010-09-07 21:37:52


On Sun, 05 Sep 2010 18:05:36 +0400, Boris Schaeling <boris_at_[hidden]>
wrote:

> As there has been a lot of feedback I started to update the library.
> There is now a new version available at
> <http://www.highscore.de/boost/gsoc2010/process.zip> (and of course in
> Subversion at
> <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/>).
>
> What has been changed?
>
> [snip]
>
> I'd appreciate if you could comment on the changes. Especially have a
> look at:
>
> * this signature: std::pair<handle, handle> (bool).

It would be better if delegate would accept fileno instead of bool
indicating input/output. bp::behavior::inherit seems almost
indistinguishable from the redirect_to.

> Shall we use std::pair<handle, handle> to return two handles for the
> child and parent process or a struct with member variables called
> parent_end and child_end? Shall we use a bool to indicate whether an
> input or output stream is configured or something else like an
> enumeration? I think it all depends on how many developers want to
> define new stream behaviors and if it's worth to make the signature a
> bit more self-explanatory?
>
> * the implementation of async_pipe on Windows. Are return values of
> Windows functions checked correctly?

Nod.

> Is the return value of UuidCreateSequential() and UuidToStringA() the
> error code or must GetLastError() be called?

The former, and it should be passed to the system_error's constructor.
system_category should be fine for it.

> Is it OK if we require developers to link against rpcrt4.lib on Windows
> because async_pipe uses UuidCreateSequential() now?

Nod.

> * the example at
> <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/redirect_to.cpp>.
> It uses a user-defined behavior class to redirect the child's standard
> error stream to the standard output stream. As the behavior of the
> standard error stream must somehow access the handle of the standard
> output stream I create the pipe myself before I set the stream
> behaviors. As the pipe must be used for the standard output stream
> though I define a forward function which I can bind to the respective
> delegate. Anyway, please have a look at the example and think about
> whether it makes sense or if there is anything to simplify. :-)

IMO, the redirect_to_stderr behavior must be provided by the library
as it is a common use case. Unfortunately, it breaks the interface (((.

> What still has to be done?
>
> * The context class still has a default constructor only which
> initializes the stream behaviors to inherit standard streams. This is a
> problem for Windows GUI applications as they don't have standard streams
> (or at least they are closed). While the Named Constructor Idiom has
> been proposed what exactly should the done? Should the default
> constructor be private and developers must explicitly use a factory
> function to create a context object? Shall the factory function
> initialize all standard streams with the same behavior? Shall we provide
> a factory function which expects three parameters to initialize the
> standard streams? What's convenient and flexible and not too confusing?

I expect that if some std stream is closed in the calling process,
the default constructor of the context class doesn't throw anything.
As I see it, the bp::handle should have a "dont-close" flag and
bp::behavior::inherit shouldn't call dup() or DuplicateHandle().

> [snip]


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