Return-Path: Delivered-To: apmail-apr-dev-archive@www.apache.org Received: (qmail 28794 invoked from network); 6 Aug 2008 04:00:53 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 6 Aug 2008 04:00:53 -0000 Received: (qmail 3167 invoked by uid 500); 6 Aug 2008 04:00:51 -0000 Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 3098 invoked by uid 500); 6 Aug 2008 04:00:51 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Id: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 3087 invoked by uid 99); 6 Aug 2008 04:00:51 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 05 Aug 2008 21:00:51 -0700 X-ASF-Spam-Status: No, hits=-0.0 required=10.0 tests=SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of bojan@rexursive.com designates 203.171.74.242 as permitted sender) Received: from [203.171.74.242] (HELO beauty.rexursive.com) (203.171.74.242) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 06 Aug 2008 03:59:56 +0000 Received: from [10.1.120.24] (shrek.rexursive.com [10.1.120.24]) by beauty.rexursive.com (Postfix) with ESMTP id 70B0940369 for ; Wed, 6 Aug 2008 14:00:21 +1000 (EST) Subject: Re: To O_CLOEXEC or not? From: Bojan Smojver To: APR Development List In-Reply-To: <4898B0AB.1000601@rowe-clan.net> References: <1217901131.25598.58.camel@shrek.rexursive.com> <4898B0AB.1000601@rowe-clan.net> Content-Type: multipart/mixed; boundary="=-cFgaSaQEtcsY4rQ0prXz" Date: Wed, 06 Aug 2008 14:00:21 +1000 Message-Id: <1217995221.25598.67.camel@shrek.rexursive.com> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 (2.22.3.1-1.fc9) X-Virus-Checked: Checked by ClamAV on apache.org --=-cFgaSaQEtcsY4rQ0prXz Content-Type: text/plain Content-Transfer-Encoding: 7bit On Tue, 2008-08-05 at 14:57 -0500, William A. Rowe, Jr. wrote: > You have it upside down though, apr_file_inherit_[un]set should toggle > the CLOEXEC flag. I was thinking something like this (untested). -- Bojan --=-cFgaSaQEtcsY4rQ0prXz Content-Disposition: attachment; filename=apr-o_cloexec.patch Content-Type: text/x-patch; name=apr-o_cloexec.patch; charset=UTF-8 Content-Transfer-Encoding: 7bit Index: file_io/unix/open.c =================================================================== --- file_io/unix/open.c (revision 683098) +++ file_io/unix/open.c (working copy) @@ -125,6 +125,12 @@ oflags |= O_BINARY; } #endif + +#ifdef O_CLOEXEC + if (!(flag & APR_FILE_NOCLEANUP)) { + oflags |= O_CLOEXEC; + } +#endif #if APR_HAS_LARGE_FILES && defined(_LARGEFILE64_SOURCE) oflags |= O_LARGEFILE; @@ -324,8 +330,30 @@ return apr_file_open_flags_stdin(thefile, 0, pool); } -APR_IMPLEMENT_INHERIT_SET(file, flags, pool, apr_unix_file_cleanup) +/* Implemented by hand in order to take advantage of O_CLOEXEC where + * available. */ +APR_DECLARE(apr_status_t) apr_file_inherit_set(apr_file_t *thefile) +{ + if (thefile->flags & APR_FILE_NOCLEANUP) { + return APR_EINVAL; + } + if (!(thefile->flags & APR_INHERIT)) { +#ifdef O_CLOEXEC + int flags = fcntl(thefile->filedes, F_GETFD); + flags &= ~(FD_CLOEXEC); + + fcntl(thefile->filedes, F_SETFD, flags); +#endif + thefile->flags |= APR_INHERIT; + apr_pool_child_cleanup_set(thefile->pool, + (void *)thefile, + apr_unix_file_cleanup, + apr_pool_cleanup_null); + } + return APR_SUCCESS; +} + /* We need to do this by hand instead of using APR_IMPLEMENT_INHERIT_UNSET * because the macro sets both cleanups to the same function, which is not * suitable on Unix (see PR 41119). */ @@ -335,6 +363,13 @@ return APR_EINVAL; } if (thefile->flags & APR_INHERIT) { +#ifdef O_CLOEXEC + int flags = fcntl(thefile->filedes, F_GETFD); + + flags |= FD_CLOEXEC; + + fcntl(thefile->filedes, F_SETFD, flags); +#endif thefile->flags &= ~APR_INHERIT; apr_pool_child_cleanup_set(thefile->pool, (void *)thefile, --=-cFgaSaQEtcsY4rQ0prXz--