Joe Orton wrote: > 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);
I'm sorry the issue is confused. THREE cases, this is a third that should be no problem... > 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! It turns out that all flavors have always been willing to take a parent NULL handle. I'm speaking of the NULL+NULL case, where the combination of double nulls, instead of assigning a file handle, create a pipe on the fly... >> 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. In APR 1.3? No, let's decide on a behavior, and implement it consistently. (I'm *not* talking about what APR 1.2.x should/should-not be doing). Bill
