On Wed, Mar 05, 2003 at 08:03:51PM +0100, Christian Kratzer wrote: > Hi, > > On Wed, 5 Mar 2003, Joe Orton wrote: > > > Pipes created by the Unix implementation of apr_file_pipe_create are > > inherited by default, but you can't make them uninherited without > > registering a custom cleanup handler. Any objections to this patch, > > which lets apr_file_inherit_unset work on a pipe for Unix? (it looks > > like it might already work on Win32). > > > > Alternatively I wonder whether it would be better to not leak pipes by > > default, though presumably it was done this way for a reason... > [snipp] > > + (*in)->flags = (*out)->flags = APR_INHERIT; > > With this you are setting the inherit flag instead of clearing it. > Should this not rather be the following > > + (*in)->flags &= ~APR_INHERIT); > + (*out)->flags &= ~APR_INHERIT); > > which would clear APR_INHERIT in both flags.
The APR_INHERIT flag needs to be set, since the current code does allow pipes to be inherited by default. If the APR_INHERIT flag is not set, apr_file_inherit_unset has no effect. To answer Bill's question as well, flags is zero at this point in the function, so a bitop is not need, just an assignment. > But why not just register apr_unix_file_cleanup for cleanup like in Indeed, I wonder it was implement this way too. I think the change you suggest is a little more intrusive than mine, as currently some callers are compensating for the fact that no child_cleanup is registered. I also wonder why the code goes to these lengths when on Unix setting the CLOEXEC flag would probably suffice. > --- httpd-2.0.44/srclib/apr/file_io/unix/pipe.c.orig Sun Mar 2 00:54:10 > 2003 > +++ httpd-2.0.44/srclib/apr/file_io/unix/pipe.c Sun Mar 2 00:54:36 2003 > @@ -225,9 +225,9 @@ > #endif > > apr_pool_cleanup_register((*in)->pool, (void *)(*in), > apr_unix_file_cleanup, > - apr_pool_cleanup_null); > + apr_unix_file_cleanup); > apr_pool_cleanup_register((*out)->pool, (void *)(*out), > apr_unix_file_cleanup, > - apr_pool_cleanup_null); > + apr_unix_file_cleanup); > return APR_SUCCESS; > } > > would this not be more to the point of getting the descriptors closed on exec. > > Greetings > Christian > > -- > CK Software GmbH > Christian Kratzer, Schwarzwaldstr. 31, 71131 Jettingen > Email: [EMAIL PROTECTED] > Phone: +49 7452 889-135 Open Software Solutions, Network Security > Fax: +49 7452 889-136 FreeBSD spoken here! >
