On Wed, 08 Sep 2010 03:37:52 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]
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.
Which fileno should be passed? stdin/stdout/stderr of the parent process? Maybe we have a similar idea because I also think it would be better to distinguish standard output and standard error. This could be done with an enumeration and could maybe help to create a better redirect_to stream behavior. The problem with redirect_to though is that it depends on another stream behavior. I don't know whether Boost.Process should support dependencies between stream behaviors? Here is some pseudocode which could work somehow: ---------- enum stream_type { in, out, err }; class redirect_to { public: redirect_to(stream_type to) : to_(to) { } // delegate gets reference to context object to access // other stream behaviors pair<handle, handle> operator()(stream_type st, context &ctx) const { if (to_ == out && st == err) { // init_stdout_behavior() calls stdout_behavior() with correct // parameters and caches handles handle child_stdout = ctx.init_stdout_behavior().first; return make_pair(child_stdout, handle()); } else if ... } private: stream_type to_; }; context ctx; ctx.stdout_behavior = pipe(); ctx.stderr_behavior = redirect_to(out); ---------- init_stdout_behavior() would need to make sure that stream behaviors are initialized only once (not that there are multiple pipes created). But then it's difficult again to reuse a context object as it would need to be reset explicitly. But this could be done in create_child() again. - Comments?
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().
The reason why bp::behavior::inherit calls dup()/DuplicateHandle() is that a parent closes a child's handles (and a child a parent's handles; this is done in create_child()). Otherwise both processes would have a read and write end open when a pipe is used. Without a dup()/DuplicateHandle() a parent would close its own standard streams if it wants the child to inherit them. I think it also makes logical sense to automatically close the streams of the other process? 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. By default it throws - but it doesn't when used by the default constructor of context? Anyway, enough ideas for now. :) Boris