On 09/12/2010 08:45 AM, Boris Schaeling wrote:
On Sun, 12 Sep 2010 01:03:59 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]Suppose you want to map the child's file descriptor 3 to what the parent's file descriptor 1 is, and you also want the child's file descriptor 1 to be set to something else. You configure the child's file descriptor 1 using the library's interface. Since context::setup is called after the child's file descriptor 1 has been remapped, you no longer have access to the parent's file descriptor 1, unless you took care to dup it previously (and ensure that you are returned something > 2).
Yes, this is what create_child() does with the standard streams. And I agree you probably have to do the same (calling fcntl(..., F_DUPFD, 3)). But I'm not sure if this POSIX-only problem which can be rather easily solved by the user and is only an issue when a fd > 2 should be mapped to a fd < 3 justifies to change the interface of context (at least I don't expect it will become easier to use)? I would agree that this would definitely need to be documented.
Mapping an fd > 2 to an existing fd <= 2 was an extreme example where the Boost Process code was actively interfering, but in fact the naive approach of dup2 followed by close will get you in trouble quite easily. (Suppose you want to assign to both 3 and 4, and it happens that the existing fds are assigned to 4 and 3 respectively, or you want to assign something to e.g. 3 and it happens to already be 3, so you end up closing it, etc.) From my perspective, this means there is a clear need (by any users that need to do mappings > 3) for a library to handle the file descriptor mapping. You make the argument that boost process is not the right library, but if Boost.Process is being used, it seems clear that the functionality could only be provided by Boost.Process. (To have a convenient interface, I think it would clearly need to be coupled with Boost.Process.)
But I'm still not sure whether Boost.Process should provide a posix_context class which supports the scenario above out of the box (I can already hear users asking "if the library supports X on POSIX why not Y?"). If we could somehow draw a line between "good" and "bad" platform-specific features it will be much easier for maintainers later. Maybe a direct question helps: Why should Boost.Process support fds > 2 out of the box but not chroot for example (there was a string member variable in context in previous versions to easily set the root directory)? Or your preference is just the opposite and you like to see additional platform-specific classes which support as many platform-specific features as possible?
I certainly don't want to see Boost.Process providing a way to do arbitrary POSIX things like setuid, etc. A key distinction between setuid, chroot, etc. is that they require only a single system call, so that invoking those functions directly from the callback is practically the optimal interface anyway. I would like to see a more convenient C++ interface to POSIX calls generally, but that can all be done completely independently from Boost.Process (and then potentially used from the callback function). With file descriptor redirection, I think catering more towards POSIX makes sense, because I think it is much more likely to be used on POSIX than on Windows. (My impression is that std{in,out,err} are not used very extensively on Windows at all, and doing any sort of fancy redirection is even rarer.) In general, I think it is good to keep libraries, interfaces, etc. as simple and decoupled as possible, provided that you don't sacrifice power or efficiency in the interface. If you want Boost.Process to be the _the_ c++ interface for child process creation (and really, that is what "boost." implies), then it is important to ensure that all but the most extreme cases can be handled in a useful way (e.g. either the library directly provides a convenient way to do what you want, or the library can be conveniently used in combination with some other library to do what you want), as opposed to having to bypass the library or sacrifice convenience to work with the library.