On Thu, 09 Sep 2010 20:02:54 -0700 Jeremy Maitin-Shepard <jeremy@jeremyms.com> 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