On Sun, 05 Sep 2010 18:05:36 +0400, Boris Schaeling <boris@highscore.de> 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]