On Tue, Oct 02, 2007 at 01:48:58PM -0500, William Rowe wrote: > Joe Orton wrote: > >> > >> The whole code seems fragile, why we even allow (NULL, NULL) syntax > >> which is not supported on any other platform. If a pipe is desired, > >> that is what apr_procattr_io_set is for. > > > > The interface guarantees of this code escape me too. I certainly agree > > that the the way this works in the common case by creating a pipe() then > > dup2's the passed-in fds onto each end of that pipe is very silly, not > > to mention inefficient. > > > > Do you mean NULL child_in and parent_in arguments, or attr fields? It's > > not clear to me why NULL arguments are handled given that the API > > description explicitly states all such arguments "Must be a valid file", > > though I note the trunk log.c has been changed to rely on that working. > > It (log.c) was?
r568326 introduced a call with a NULL parent_err: + rc = apr_procattr_child_err_set(procattr, errfile, NULL); Also perhaps (not very) interesting to note that of the three _child_*_set() functions, _in_set says "Set the child_in and/or parent_in" whereas the other two lack the "or" clause... blech! > IMHO, apr_procattr_io_set() provides the mechanism for creating child pipes, > while apr_procattr_stdxxx_set() provides the mechanism for injecting the > programmer's choice of API's. > > I'd suggest we document NULL,NULL as a supported option if it is, turn around > and mark it deprecated, and plan to remove that interface in APR 2.0. Given that the NULL, NULL case already has different behaviour across platforms (Win32: noop?, Unix: creates a pipe) I'd just leave this as undocumented. joe
