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

Subject: Re: [boost] [gsoc] Boost.Process done
From: Ilya Sokolov (ilyasokol_at_[hidden])
Date: 2010-09-10 18:49:19

On Thu, 09 Sep 2010 20:02:54 -0700
Jeremy Maitin-Shepard <jeremy_at_[hidden]> wrote:

> This is a library of significant interest to me, since particularly 
> under POSIX child process creation and management is rather
> complicated to do right.  Based on looking at the documentation on
> the website, and browsing through the discussion that has taken place
> on this list, I have some comments.  I understand that the website
> does not reflect the planned changes to the stream behavior
> configuration interface.
> 
> 
> - I suggest some interface changes for the facilities for retrieving
> the parent side of pipes created for stdin, stdout, and stderr of the
> child:
>    - The std::istream/std::ostream interface should be decoupled from 
> the facility for retrieving the file descriptor (int) or Windows
> HANDLE, to avoid the overhead of allocating a streambuf

Could be solved using some form of lazy initialization

> for those
> users that wish to access the file descriptor/HANDLE directly or use
> it with asio. Instead, the std::istream/std::ostream interface to a
> file descriptor/HANDLE could be provided as a separate, completely 
> independent facility

Boost.IOStream?

> (designed so that exactly the same syntax could
> be used on both Windows and POSIX), and likewise a separate facility
> for providing an asio interface to a file descriptor/HANDLE (which
> also allows exactly the same syntax to be used on POSIX and Windows).
>    - It seems it would be cleaner

Not sure

> for the parent side of these pipes
> to be returned in the course of specifying that pipes should be used
> (in the current terminology, in the course of specifying the pipe
> stream behavior).
>
> [snip]
> 
> - As was observed in the discussion regarding setting up 
> stdin/stdout/stderr file descriptors on POSIX, it is somewhat tricky
> to correctly set up child file descriptors.  As I believe it is
> fairly common on POSIX to want to set up child file descriptors other
> than 0, 1, and 2, it would be best for the library to provide
> explicit support for this, rather than forcing the user to implement
> it manually in context::setup.  A reasonable interface might be e.g.:
>    context::map_file_descriptor(int child_fileno, file_descriptor_t 
> existing_descriptor);

Not sure what it means (. The most obvious extension to the current
interface is:

context::set_behavior(fileno fn, function<...> b);

> Potentially this same interface could be used on Windows, with 
> child_fileno only valid in the range 0 to 2 (and with appropriate 
> constants defined), and could thus serve as the lowest level
> interface

Why is it needed?

on top of which other behaviors like pipe, redirect, etc.
> could be implemented.
> 
> - On POSIX, the precise stage in the child process setup at which 
> context::setup is called should be documented.

+1

> - One potential issue is that the Windows extension interface only 
> exposes a (relatively) small subset of the functionality that can be 
> configured on Windows.  There seem to be numerous flags to
> CreateProcess that are not exposed by Boost.Process, and there are
> also two other functions, CreateProcessAsUser and
> CreateProcessWithLoginW, which allow specifying some additional
> options.

The interface could be made so complex that winapi would become easier.

> I am not a Windows programmer, so I don't know how
> important this additional functionality is in practice, but it seems
> it would be unfortunate for users that would like to use other
> conveniences provided by Boost.Process (such as pipe setup, perhaps)
> to have to revert to invoking the Windows API directly in order to
> make use of additional functionality not exposed.  In contrast, it is
> hard to imagine the POSIX extension interface being insufficient.
> I'm not sure how this could be solved, exactly.
> 
> - I think it would be cleaner for the POSIX and Windows extension 
> interfaces to be in the form of a callback function (templated or 
> boost::function), rather than a subclass of context that overrides
> the setup function.

+1