You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by bo...@apache.org on 2009/02/26 03:41:22 UTC
svn commit: r747990 - in /apr/apr/trunk: ./ file_io/netware/ file_io/unix/
include/arch/unix/ network_io/unix/ poll/unix/
Author: bojan
Date: Thu Feb 26 02:41:21 2009
New Revision: 747990
URL: http://svn.apache.org/viewvc?rev=747990&view=rev
Log:
Set CLOEXEC flags where appropriate. Either use new O_CLOEXEC flag and
associated functions, such as dup3(), accept4(), epoll_create1() etc., or
simply set CLOEXEC flag using fcntl().
PR 46425.
Modified:
apr/apr/trunk/configure.in
apr/apr/trunk/file_io/netware/mktemp.c
apr/apr/trunk/file_io/unix/filedup.c
apr/apr/trunk/file_io/unix/mktemp.c
apr/apr/trunk/file_io/unix/open.c
apr/apr/trunk/include/arch/unix/apr_arch_inherit.h
apr/apr/trunk/network_io/unix/sockets.c
apr/apr/trunk/poll/unix/epoll.c
apr/apr/trunk/poll/unix/kqueue.c
apr/apr/trunk/poll/unix/pollset.c
apr/apr/trunk/poll/unix/port.c
Modified: apr/apr/trunk/configure.in
URL: http://svn.apache.org/viewvc/apr/apr/trunk/configure.in?rev=747990&r1=747989&r2=747990&view=diff
==============================================================================
--- apr/apr/trunk/configure.in (original)
+++ apr/apr/trunk/configure.in Thu Feb 26 02:41:21 2009
@@ -775,6 +775,23 @@
AC_DEFINE([HAVE_EPOLL], 1, [Define if the epoll interface is supported])
fi
+dnl ----------------------------- Checking for extended file descriptor handling
+AC_CHECK_FUNCS(dup3 accept4 epoll_create1)
+
+AC_CACHE_CHECK([for SOCK_CLOEXEC support], [apr_cv_sock_cloexec],
+[AC_TRY_RUN([
+#include <sys/types.h>
+#include <sys/socket.h>
+
+int main()
+{
+ return socket(AF_INET, SOCK_STREAM|SOCK_CLOEXEC, 0) == -1;
+}], [apr_cv_sock_cloexec=yes], [apr_cv_sock_cloexec=no], [apr_cv_sock_cloexec=no])])
+
+if test "$apr_cv_sock_cloexec" = "yes"; then
+ AC_DEFINE([HAVE_SOCK_CLOEXEC], 1, [Define if the SOCK_CLOEXEC flag is supported])
+fi
+
dnl ----------------------------- Checking for missing POSIX thread functions
AC_CHECK_FUNCS([getpwnam_r getpwuid_r getgrnam_r getgrgid_r])
Modified: apr/apr/trunk/file_io/netware/mktemp.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/netware/mktemp.c?rev=747990&r1=747989&r2=747990&view=diff
==============================================================================
--- apr/apr/trunk/file_io/netware/mktemp.c (original)
+++ apr/apr/trunk/file_io/netware/mktemp.c Thu Feb 26 02:41:21 2009
@@ -19,6 +19,7 @@
#include "apr_strings.h" /* prototype of apr_mkstemp() */
#include "apr_arch_file_io.h" /* prototype of apr_mkstemp() */
#include "apr_portable.h" /* for apr_os_file_put() */
+#include "apr_arch_inherit.h"
#include <stdlib.h> /* for mkstemp() - Single Unix */
@@ -43,6 +44,7 @@
if (!(flags & APR_FILE_NOCLEANUP)) {
+ APR_SET_FD_CLOEXEC((*fp)->filedes);
apr_pool_cleanup_register((*fp)->pool, (void *)(*fp),
apr_unix_file_cleanup,
apr_unix_child_file_cleanup);
Modified: apr/apr/trunk/file_io/unix/filedup.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/filedup.c?rev=747990&r1=747989&r2=747990&view=diff
==============================================================================
--- apr/apr/trunk/file_io/unix/filedup.c (original)
+++ apr/apr/trunk/file_io/unix/filedup.c Thu Feb 26 02:41:21 2009
@@ -24,14 +24,25 @@
apr_file_t *old_file, apr_pool_t *p,
int which_dup)
{
- int rv;
+ int rv, flags = 0;
if (which_dup == 2) {
if ((*new_file) == NULL) {
/* We can't dup2 unless we have a valid new_file */
return APR_EINVAL;
}
+#ifdef HAVE_DUP3
+ if (!(old_file->flags & APR_INHERIT))
+ flags |= O_CLOEXEC;
+ rv = dup3(old_file->filedes, (*new_file)->filedes, flags);
+#else
rv = dup2(old_file->filedes, (*new_file)->filedes);
+ if (!(old_file->flags & APR_INHERIT)) {
+ if (rv == -1)
+ return errno;
+ APR_SET_FD_CLOEXEC((*new_file)->filedes);
+ }
+#endif
} else {
rv = dup(old_file->filedes);
}
Modified: apr/apr/trunk/file_io/unix/mktemp.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/mktemp.c?rev=747990&r1=747989&r2=747990&view=diff
==============================================================================
--- apr/apr/trunk/file_io/unix/mktemp.c (original)
+++ apr/apr/trunk/file_io/unix/mktemp.c Thu Feb 26 02:41:21 2009
@@ -51,6 +51,7 @@
#include "apr_strings.h" /* prototype of apr_mkstemp() */
#include "apr_arch_file_io.h" /* prototype of apr_mkstemp() */
#include "apr_portable.h" /* for apr_os_file_put() */
+#include "apr_arch_inherit.h"
#ifndef HAVE_MKSTEMP
@@ -203,6 +204,7 @@
(*fp)->fname = apr_pstrdup(p, template);
if (!(flags & APR_FILE_NOCLEANUP)) {
+ APR_SET_FD_CLOEXEC(fd);
apr_pool_cleanup_register((*fp)->pool, (void *)(*fp),
apr_unix_file_cleanup,
apr_unix_child_file_cleanup);
Modified: apr/apr/trunk/file_io/unix/open.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/open.c?rev=747990&r1=747989&r2=747990&view=diff
==============================================================================
--- apr/apr/trunk/file_io/unix/open.c (original)
+++ apr/apr/trunk/file_io/unix/open.c Thu Feb 26 02:41:21 2009
@@ -127,7 +127,15 @@
oflags |= O_BINARY;
}
#endif
-
+
+#ifdef O_CLOEXEC
+ /* Introduced in Linux 2.6.23. Silently ignored on earlier Linux kernels.
+ */
+ if (!(flag & APR_FILE_NOCLEANUP)) {
+ oflags |= O_CLOEXEC;
+}
+#endif
+
#if APR_HAS_LARGE_FILES && defined(_LARGEFILE64_SOURCE)
oflags |= O_LARGEFILE;
#elif defined(O_LARGEFILE)
@@ -155,6 +163,9 @@
if (fd < 0) {
return errno;
}
+ if (!(flag & APR_FILE_NOCLEANUP)) {
+ APR_SET_FD_CLOEXEC(fd);
+ }
(*new) = (apr_file_t *)apr_pcalloc(pool, sizeof(apr_file_t));
(*new)->pool = pool;
@@ -337,6 +348,7 @@
return APR_EINVAL;
}
if (thefile->flags & APR_INHERIT) {
+ APR_SET_FD_CLOEXEC(thefile->filedes);
thefile->flags &= ~APR_INHERIT;
apr_pool_child_cleanup_set(thefile->pool,
(void *)thefile,
Modified: apr/apr/trunk/include/arch/unix/apr_arch_inherit.h
URL: http://svn.apache.org/viewvc/apr/apr/trunk/include/arch/unix/apr_arch_inherit.h?rev=747990&r1=747989&r2=747990&view=diff
==============================================================================
--- apr/apr/trunk/include/arch/unix/apr_arch_inherit.h (original)
+++ apr/apr/trunk/include/arch/unix/apr_arch_inherit.h Thu Feb 26 02:41:21 2009
@@ -27,6 +27,12 @@
if (the##name->flag & APR_FILE_NOCLEANUP) \
return APR_EINVAL; \
if (!(the##name->flag & APR_INHERIT)) { \
+ int flags = fcntl(the##name->name##des, F_GETFD); \
+ if (flags == -1) \
+ return errno; \
+ flags &= ~(FD_CLOEXEC); \
+ if (fcntl(the##name->name##des, F_SETFD, flags) == -1) \
+ return errno; \
the##name->flag |= APR_INHERIT; \
apr_pool_child_cleanup_set(the##name->pool, \
(void *)the##name, \
@@ -35,12 +41,23 @@
return APR_SUCCESS; \
}
+#define APR_SET_FD_CLOEXEC(fd) \
+do { \
+ int flags; \
+ if ((flags = fcntl(fd, F_GETFD)) == -1) \
+ return errno; \
+ flags |= FD_CLOEXEC; \
+ if (fcntl(fd, F_SETFD, flags) == -1) \
+ return errno; \
+} while (0)
+
#define APR_IMPLEMENT_INHERIT_UNSET(name, flag, pool, cleanup) \
apr_status_t apr_##name##_inherit_unset(apr_##name##_t *the##name) \
{ \
if (the##name->flag & APR_FILE_NOCLEANUP) \
return APR_EINVAL; \
if (the##name->flag & APR_INHERIT) { \
+ APR_SET_FD_CLOEXEC(the##name->name##des); \
the##name->flag &= ~APR_INHERIT; \
apr_pool_child_cleanup_set(the##name->pool, \
(void *)the##name, \
Modified: apr/apr/trunk/network_io/unix/sockets.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/network_io/unix/sockets.c?rev=747990&r1=747989&r2=747990&view=diff
==============================================================================
--- apr/apr/trunk/network_io/unix/sockets.c (original)
+++ apr/apr/trunk/network_io/unix/sockets.c Thu Feb 26 02:41:21 2009
@@ -108,9 +108,13 @@
apr_status_t apr_socket_create(apr_socket_t **new, int ofamily, int type,
int protocol, apr_pool_t *cont)
{
- int family = ofamily;
+ int family = ofamily, flags = 0;
int oprotocol = protocol;
+#ifdef HAVE_SOCK_CLOEXEC
+ flags |= SOCK_CLOEXEC;
+#endif
+
if (family == APR_UNSPEC) {
#if APR_HAVE_IPV6
family = APR_INET6;
@@ -126,19 +130,19 @@
alloc_socket(new, cont);
#ifndef BEOS_R5
- (*new)->socketdes = socket(family, type, protocol);
+ (*new)->socketdes = socket(family, type|flags, protocol);
#else
/* For some reason BeOS R5 has an unconventional protocol numbering,
* so we need to translate here. */
switch (protocol) {
case 0:
- (*new)->socketdes = socket(family, type, 0);
+ (*new)->socketdes = socket(family, type|flags, 0);
break;
case APR_PROTO_TCP:
- (*new)->socketdes = socket(family, type, IPPROTO_TCP);
+ (*new)->socketdes = socket(family, type|flags, IPPROTO_TCP);
break;
case APR_PROTO_UDP:
- (*new)->socketdes = socket(family, type, IPPROTO_UDP);
+ (*new)->socketdes = socket(family, type|flags, IPPROTO_UDP);
break;
case APR_PROTO_SCTP:
default:
@@ -151,7 +155,7 @@
#if APR_HAVE_IPV6
if ((*new)->socketdes < 0 && ofamily == APR_UNSPEC) {
family = APR_INET;
- (*new)->socketdes = socket(family, type, protocol);
+ (*new)->socketdes = socket(family, type|flags, protocol);
}
#endif
@@ -160,6 +164,10 @@
}
set_socket_vars(*new, family, type, oprotocol);
+#ifndef HAVE_SOCK_CLOEXEC
+ APR_SET_FD_CLOEXEC((*new)->socketdes);
+#endif
+
(*new)->timeout = -1;
(*new)->inherit = 0;
apr_pool_cleanup_register((*new)->pool, (void *)(*new), socket_cleanup,
@@ -218,7 +226,11 @@
sa.salen = sizeof(sa.sa);
+#ifdef HAVE_ACCEPT4
+ s = accept4(sock->socketdes, (struct sockaddr *)&sa.sa, &sa.salen, SOCK_CLOEXEC);
+#else
s = accept(sock->socketdes, (struct sockaddr *)&sa.sa, &sa.salen);
+#endif
if (s < 0) {
return errno;
@@ -300,6 +312,10 @@
(*new)->local_interface_unknown = 1;
}
+#ifndef HAVE_ACCEPT4
+ APR_SET_FD_CLOEXEC((*new)->socketdes);
+#endif
+
(*new)->inherit = 0;
apr_pool_cleanup_register((*new)->pool, (void *)(*new), socket_cleanup,
socket_cleanup);
Modified: apr/apr/trunk/poll/unix/epoll.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/epoll.c?rev=747990&r1=747989&r2=747990&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/epoll.c (original)
+++ apr/apr/trunk/poll/unix/epoll.c Thu Feb 26 02:41:21 2009
@@ -21,6 +21,7 @@
#include "apr_arch_file_io.h"
#include "apr_arch_networkio.h"
#include "apr_arch_poll_private.h"
+#include "apr_arch_inherit.h"
#if defined(HAVE_EPOLL)
@@ -95,12 +96,20 @@
apr_status_t rv;
int fd;
+#ifdef HAVE_EPOLL_CREATE1
+ fd = epoll_create1(EPOLL_CLOEXEC);
+#else
fd = epoll_create(size);
+#endif
if (fd < 0) {
pollset->p = NULL;
return apr_get_netos_error();
}
+#ifndef HAVE_EPOLL_CREATE1
+ APR_SET_FD_CLOEXEC(fd);
+#endif
+
pollset->p = apr_palloc(p, sizeof(apr_pollset_private_t));
#if APR_HAS_THREADS
if ((flags & APR_POLLSET_THREADSAFE) &&
@@ -319,15 +328,23 @@
{
int fd;
+#ifdef HAVE_EPOLL_CREATE1
+ fd = epoll_create1(EPOLL_CLOEXEC);
+#else
fd = epoll_create(size);
+#endif
if (fd < 0) {
return apr_get_netos_error();
}
+
+#ifndef HAVE_EPOLL_CREATE1
+ APR_SET_FD_CLOEXEC(fd);
+#endif
pollcb->fd = fd;
pollcb->pollset.epoll = apr_palloc(p, size * sizeof(struct epoll_event));
- apr_pool_cleanup_register(p, pollcb, cb_cleanup, cb_cleanup);
+ apr_pool_cleanup_register(p, pollcb, cb_cleanup, apr_pool_cleanup_null);
return APR_SUCCESS;
}
Modified: apr/apr/trunk/poll/unix/kqueue.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/kqueue.c?rev=747990&r1=747989&r2=747990&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/kqueue.c (original)
+++ apr/apr/trunk/poll/unix/kqueue.c Thu Feb 26 02:41:21 2009
@@ -21,6 +21,7 @@
#include "apr_arch_file_io.h"
#include "apr_arch_networkio.h"
#include "apr_arch_poll_private.h"
+#include "apr_arch_inherit.h"
#ifdef HAVE_KQUEUE
@@ -98,6 +99,8 @@
return apr_get_netos_error();
}
+ APR_SET_FD_CLOEXEC(pollset->p->kqueue_fd);
+
pollset->p->result_set = apr_palloc(p, size * sizeof(apr_pollfd_t));
APR_RING_INIT(&pollset->p->query_ring, pfd_elem_t, link);
@@ -312,10 +315,12 @@
if (fd < 0) {
return apr_get_netos_error();
}
-
+
+ APR_SET_FD_CLOEXEC(fd);
+
pollcb->fd = fd;
pollcb->pollset.ke = (struct kevent *)apr_pcalloc(p, size * sizeof(struct kevent));
- apr_pool_cleanup_register(p, pollcb, cb_cleanup, cb_cleanup);
+ apr_pool_cleanup_register(p, pollcb, cb_cleanup, apr_pool_cleanup_null);
return APR_SUCCESS;
}
Modified: apr/apr/trunk/poll/unix/pollset.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/pollset.c?rev=747990&r1=747989&r2=747990&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/pollset.c (original)
+++ apr/apr/trunk/poll/unix/pollset.c Thu Feb 26 02:41:21 2009
@@ -26,6 +26,7 @@
#include "apr_arch_file_io.h"
#include "apr_arch_networkio.h"
#include "apr_arch_poll_private.h"
+#include "apr_arch_inherit.h"
static apr_pollset_method_e pollset_default_method = POLLSET_DEFAULT_METHOD;
@@ -87,6 +88,10 @@
fd.reqevents = APR_POLLIN;
fd.desc_type = APR_POLL_FILE;
fd.desc.f = pollset->wakeup_pipe[0];
+
+ APR_SET_FD_CLOEXEC(pollset->wakeup_pipe[0]->filedes);
+ APR_SET_FD_CLOEXEC(pollset->wakeup_pipe[1]->filedes);
+
/* Add the pipe to the pollset
*/
return apr_pollset_add(pollset, &fd);
Modified: apr/apr/trunk/poll/unix/port.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/port.c?rev=747990&r1=747989&r2=747990&view=diff
==============================================================================
--- apr/apr/trunk/poll/unix/port.c (original)
+++ apr/apr/trunk/poll/unix/port.c Thu Feb 26 02:41:21 2009
@@ -22,6 +22,7 @@
#include "apr_arch_file_io.h"
#include "apr_arch_networkio.h"
#include "apr_arch_poll_private.h"
+#include "apr_arch_inherit.h"
#if defined(HAVE_PORT_CREATE)
@@ -125,6 +126,8 @@
return APR_ENOMEM;
}
+ APR_SET_FD_CLOEXEC(pollset->p->port_fd);
+
pollset->p->result_set = apr_palloc(p, size * sizeof(apr_pollfd_t));
APR_RING_INIT(&pollset->p->query_ring, pfd_elem_t, link);
@@ -395,8 +398,10 @@
return apr_get_netos_error();
}
+ APR_SET_FD_CLOEXEC(fd);
+
pollcb->pollset.port = apr_palloc(p, size * sizeof(port_event_t));
- apr_pool_cleanup_register(p, pollcb, cb_cleanup, cb_cleanup);
+ apr_pool_cleanup_register(p, pollcb, cb_cleanup, apr_pool_cleanup_null);
return APR_SUCCESS;
}
Re: svn commit: r747990 - in /apr/apr/trunk: ./ file_io/netware/
file_io/unix/ include/arch/unix/ network_io/unix/ poll/unix/
Posted by Bojan Smojver <bo...@rexursive.com>.
On Thu, 2009-02-26 at 02:41 +0000, bojan@apache.org wrote:
> Author: bojan
> Date: Thu Feb 26 02:41:21 2009
> New Revision: 747990
>
> URL: http://svn.apache.org/viewvc?rev=747990&view=rev
> Log:
> Set CLOEXEC flags where appropriate. Either use new O_CLOEXEC flag and
> associated functions, such as dup3(), accept4(), epoll_create1() etc., or
> simply set CLOEXEC flag using fcntl().
> PR 46425.
Please review folks. Looked good to me, it compiles on F-10 and all APR
tests still pass.
--
Bojan
Re: svn commit: r747990 - in /apr/apr/trunk: ./ file_io/netware/
file_io/unix/ include/arch/unix/ network_io/unix/ poll/unix/
Posted by Bojan Smojver <bo...@rexursive.com>.
On Thu, 2009-02-26 at 14:58 +0100, Ruediger Pluem wrote:
> IMHO doing a return in a macro that is just a block not a function
> is bad. What if the return parameter of the function is not an int?
> Furthermore it is a side effect that makes it hard to read the code.
Thanks for reviewing! I'll unroll that.
--
Bojan
Re: svn commit: r747990 - in /apr/apr/trunk: ./ file_io/netware/
file_io/unix/ include/arch/unix/ network_io/unix/ poll/unix/
Posted by Ruediger Pluem <rp...@apache.org>.
On 02/26/2009 03:41 AM, bojan@apache.org wrote:
> Author: bojan
> Date: Thu Feb 26 02:41:21 2009
> New Revision: 747990
>
> URL: http://svn.apache.org/viewvc?rev=747990&view=rev
> Log:
> Set CLOEXEC flags where appropriate. Either use new O_CLOEXEC flag and
> associated functions, such as dup3(), accept4(), epoll_create1() etc., or
> simply set CLOEXEC flag using fcntl().
> PR 46425.
>
> Modified:
> apr/apr/trunk/configure.in
> apr/apr/trunk/file_io/netware/mktemp.c
> apr/apr/trunk/file_io/unix/filedup.c
> apr/apr/trunk/file_io/unix/mktemp.c
> apr/apr/trunk/file_io/unix/open.c
> apr/apr/trunk/include/arch/unix/apr_arch_inherit.h
> apr/apr/trunk/network_io/unix/sockets.c
> apr/apr/trunk/poll/unix/epoll.c
> apr/apr/trunk/poll/unix/kqueue.c
> apr/apr/trunk/poll/unix/pollset.c
> apr/apr/trunk/poll/unix/port.c
>
> Modified: apr/apr/trunk/include/arch/unix/apr_arch_inherit.h
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/include/arch/unix/apr_arch_inherit.h?rev=747990&r1=747989&r2=747990&view=diff
> ==============================================================================
> --- apr/apr/trunk/include/arch/unix/apr_arch_inherit.h (original)
> +++ apr/apr/trunk/include/arch/unix/apr_arch_inherit.h Thu Feb 26 02:41:21 2009
> @@ -27,6 +27,12 @@
> if (the##name->flag & APR_FILE_NOCLEANUP) \
> return APR_EINVAL; \
> if (!(the##name->flag & APR_INHERIT)) { \
> + int flags = fcntl(the##name->name##des, F_GETFD); \
> + if (flags == -1) \
> + return errno; \
> + flags &= ~(FD_CLOEXEC); \
> + if (fcntl(the##name->name##des, F_SETFD, flags) == -1) \
> + return errno; \
> the##name->flag |= APR_INHERIT; \
> apr_pool_child_cleanup_set(the##name->pool, \
> (void *)the##name, \
> @@ -35,12 +41,23 @@
> return APR_SUCCESS; \
> }
>
> +#define APR_SET_FD_CLOEXEC(fd) \
> +do { \
> + int flags; \
> + if ((flags = fcntl(fd, F_GETFD)) == -1) \
> + return errno; \
> + flags |= FD_CLOEXEC; \
> + if (fcntl(fd, F_SETFD, flags) == -1) \
> + return errno; \
> +} while (0)
IMHO doing a return in a macro that is just a block not a function
is bad. What if the return parameter of the function is not an int?
Furthermore it is a side effect that makes it hard to read the code.
> +
> #define APR_IMPLEMENT_INHERIT_UNSET(name, flag, pool, cleanup) \
> apr_status_t apr_##name##_inherit_unset(apr_##name##_t *the##name) \
> { \
> if (the##name->flag & APR_FILE_NOCLEANUP) \
> return APR_EINVAL; \
> if (the##name->flag & APR_INHERIT) { \
> + APR_SET_FD_CLOEXEC(the##name->name##des); \
> the##name->flag &= ~APR_INHERIT; \
> apr_pool_child_cleanup_set(the##name->pool, \
> (void *)the##name, \
>
Regards
Rüdiger