Re: [boost] [gsoc] Boost.Process done

Subject: Re: [boost] [gsoc] Boost.Process done
From: Boris Schaeling (boris_at_[hidden])
Date: 2010-09-08 17:20:01

On Wed, 08 Sep 2010 03:37:52 +0200, Ilya Sokolov <ilyasokol_at_[hidden]>  
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