On 09/11/2010 02:47 PM, Boris Schaeling wrote:
On Sat, 11 Sep 2010 22:51:25 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
On 09/11/2010 01:32 PM, Boris Schaeling wrote:
Indeed. But as context doesn't support arbitrary file descriptors those must be configured differently anyway.
I'm suggesting that it should. From the library's perspective, there is no difference between configuring the child's file descriptors 0, 1, and 2, and configuring any other file descriptors, at least on POSIX, and given that it is useful for the user, it might as well be exposed. (Of course, on Windows, things are different, but I think you have just as elegant an interface and still allow this flexibility for POSIX while retaining portable syntax.)
I'm not really convinced of adding platform-specific features. We had POSIX- and Windows-only classes and functions in previous versions of the library (in fact that's how my Boost.Process version 0.3 looked like). The implementation of the new version is by far less bloated and much cleaner.
But maybe we can add another extension point? In fact it's not required to subclass context as create_child() is a function template. You can create another context class with new member variables and pass it to create_child(). The question is then what exactly would the extension point need to look like and how would it differ from context::setup() which is already provided today?
I agree that it is undesirable for the library to have explicit support/a wrapper interface for every single platform-specific feature. For POSIX, the existing function callback interface is exactly the right interface for supporting things like setuid, etc. (As I've mentioned before, though, I'd say the Windows extension interface is a bit lacking, though.) The exception, however, is support for file descriptor mapping, for which the POSIX extension interface is not sufficient, and which is obviously extremely closely related to handling of the "standard streams". In particular, I'd say that with the current interface, anyone that needs to do file descriptor mappings for file numbers > 2 basically shouldn't use boost.process.
Please have a look at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/file_descriptors_setup.cpp>.
There are two pipes used to create file descriptors 3 and 4 which should be inherited by the child process. As the pipe stream behavior is used explicitly the user knows if 3 and 4 should be read or write ends of the pipe. The user can pass the information to the pipe stream behavior while the context indeed can't (in this example there is still a boolean value used).
I saw that example, but it hardly seemed like a very good exposition of the convenience provided by the library. (Also, as I alluded to in some
I agree. It's a bit strange to use stream behaviors like this. But then stream behaviors were really invented to create a platform-independent way of configuring standard streams and not to support platform-specific features.
of my other example code snippets and as has been discussed a bit by others, I think that using an std::pair as the return value for those type of things is a bad idea. Instead, read_end and write_end, or parent_end and child_end, depending on the situation, should be used.)
We need both ends in create_child() as the function returns a child object. It is the one used by the parent process to possibly communicate with the child process: One end is passed to the child process and the other end remains in the parent process so it can communicate with the child process.
I was just suggesting the use of a custom struct rather than std::pair, so that you would have named members rather than first and second, to avoid confusion about the meaning of first and second. However, in some other e-mails, I suggest that boost process shouldn't itself hold on to the parent end of pipes and store them in child_process instances for the purpose of providing get_stdin(), etc. Instead, the parent end of the pipe is returned to the caller in the course of setting up the mapping, and then boost process doesn't need to do anything with it after that. I think this leads to a cleaner interface, since with the current interface, get_stdin, etc. are always present in child_process but only meaningful if you happened to have configured a pipe on the corresponding file number. Also, the current interface seems to preclude setting up a pipe between two child processes. Consider the following syntax: context ls_ctx, less_ctx; pipe(ls_ctx[bp::stdout], less_ctx[bp::stdin]); inherit(less_ctx[bp::stdout]); inherit(less_ctx[bp::stderr]); Note: bp::std{in,out,err} would be enum values, and operator[] would have an argument of enum type on Windows, and type int on POSIX, as suggested by Ilya. operator[] would return an object, e.g. fd_map_t that stores the argument and a reference to the context, and would support operator= to assign a mapping (pipe, inherit, etc. would internally invoke this operator=). So you could also do: file_descriptor_t ls_3 = output_pipe(ls_ctx[3]); // ls_ctx[3] is assigned to write end, ls_3 refers to read end. Potentially you could add some safety and/or syntactic convenience by having types like readable_file_descriptor_t, writable_file_descriptor_t, readwrite_file_descriptor_t in addition to file_descriptor_t, with the obvious implicit move conversions allowed, and explicit move conversions allowed between any of them. Then, you could have bp::pipe() return a struct e.g. pipe_t that contains both ends, and define various operators like: write_file_descriptor_t operator>>(pipe_t &&, fd_map_t); write_file_descriptor_t operator<<(fd_map_t, pipe_t &&); read_file_descriptor_t operator>>(fd_map_t, pipe_t &&); read_file_descriptor_t operator<<(pipe_t &&, fd_map_t); void operator>>(fd_map_t, write_file_descriptor_t &&); void operator<<(fd_map_t, read_file_descriptor_t &&); Then you could have: ls_ctx[bp::stdout] >> bp::pipe() >> less_ctx[bp::stdin]; write_file_descriptor_t ls_3 = ls_ctx[bp::stdout] >> bp::pipe(); The additional advantage of tagging the file descriptor types with read/write is that it can provide additional type safety if you then use them with either boost iostreams or boost asio. Note: My imagination might be running a bit too wild with these last syntactic suggestions, but in any case I think it demonstrates that you can support arbitrary file descriptor mappings on POSIX without sacrificing the quality of the portable interface. If you wanted to, you could try to prevent the user from assigning the wrong direction stream to std{out,in,err} using typing (e.g. bp::stdout,bp::stderr would have a type different from that of bp::stdin, and there would be a total of 3 versions of operator[] on POSIX, and two versions on Windows), but I don't think it is really necessary.