Kevin Cernekee wrote:
However, I don't see how any of the m4 changes address the problem
that is causing the test cases to fail: lib/getdtablesize.c still
wants to rely on sysconf(_SC_OPEN_MAX) on Android, and that call just
blindly returns 256 on all released versions of the OS. This is why
my earlier submission changed it to use getrlimit() instead.
Thanks for explaining. How about if we go even further than that patch, and
avoid the use of _SC_OPEN_MAX everywhere? I applied the attached two patches; I
think only the second one matters to you. Do they fix the problem?
>From 858bfcebf27e3a7f5e2f1f7be559f5e86b53ec05 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Fri, 20 Feb 2015 10:37:49 -0800
Subject: [PATCH 1/2] poll: fixes for large fds
* lib/poll.c (poll): Don't check directly for NFD too large.
Don't rely on undefined behavior in FD_SET when an arg exceeds
FD_SETSIZE. Always set revents afterwards, even if to zero.
* tests/test-poll.c (poll1): Set revents to -1 instead of 0,
as that makes the test a bit stricter.
---
ChangeLog | 9 ++++++++
lib/poll.c | 63 +++++++++++++++++++------------------------------------
tests/test-poll.c | 2 +-
3 files changed, 31 insertions(+), 43 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 61bd393..6f2118c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2015-02-20 Paul Eggert <egg...@cs.ucla.edu>
+
+ poll: fixes for large fds
+ * lib/poll.c (poll): Don't check directly for NFD too large.
+ Don't rely on undefined behavior in FD_SET when an arg exceeds
+ FD_SETSIZE. Always set revents afterwards, even if to zero.
+ * tests/test-poll.c (poll1): Set revents to -1 instead of 0,
+ as that makes the test a bit stricter.
+
2015-02-19 Kevin Cernekee <cerne...@google.com>
fcntl: Fix cross compiling
diff --git a/lib/poll.c b/lib/poll.c
index 36d233c..12531d9 100644
--- a/lib/poll.c
+++ b/lib/poll.c
@@ -334,26 +334,15 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)
int maxfd, rc;
nfds_t i;
-# ifdef _SC_OPEN_MAX
- static int sc_open_max = -1;
-
- if (nfd < 0
- || (nfd > sc_open_max
- && (sc_open_max != -1
- || nfd > (sc_open_max = sysconf (_SC_OPEN_MAX)))))
- {
- errno = EINVAL;
- return -1;
- }
-# else /* !_SC_OPEN_MAX */
-# ifdef OPEN_MAX
- if (nfd < 0 || nfd > OPEN_MAX)
+ if (nfd < 0)
{
errno = EINVAL;
return -1;
}
-# endif /* OPEN_MAX -- else, no check is needed */
-# endif /* !_SC_OPEN_MAX */
+ /* Don't check directly for NFD too large. Any practical use of a
+ too-large NFD is caught by one of the other checks below, and
+ checking directly for getdtablesize is too much of a portability
+ and/or performance and/or correctness hassle. */
/* EFAULT is not necessary to implement, but let's do it in the
simplest case. */
@@ -394,10 +383,17 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)
{
if (pfd[i].fd < 0)
continue;
-
+ if (maxfd < pfd[i].fd)
+ {
+ maxfd = pfd[i].fd;
+ if (FD_SETSIZE <= maxfd)
+ {
+ errno = EINVAL;
+ return -1;
+ }
+ }
if (pfd[i].events & (POLLIN | POLLRDNORM))
FD_SET (pfd[i].fd, &rfds);
-
/* see select(2): "the only exceptional condition detectable
is out-of-band data received on a socket", hence we push
POLLWRBAND events onto wfds instead of efds. */
@@ -405,18 +401,6 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)
FD_SET (pfd[i].fd, &wfds);
if (pfd[i].events & (POLLPRI | POLLRDBAND))
FD_SET (pfd[i].fd, &efds);
- if (pfd[i].fd >= maxfd
- && (pfd[i].events & (POLLIN | POLLOUT | POLLPRI
- | POLLRDNORM | POLLRDBAND
- | POLLWRNORM | POLLWRBAND)))
- {
- maxfd = pfd[i].fd;
- if (maxfd > FD_SETSIZE)
- {
- errno = EOVERFLOW;
- return -1;
- }
- }
}
/* examine fd sets */
@@ -427,18 +411,13 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)
/* establish results */
rc = 0;
for (i = 0; i < nfd; i++)
- if (pfd[i].fd < 0)
- pfd[i].revents = 0;
- else
- {
- int happened = compute_revents (pfd[i].fd, pfd[i].events,
- &rfds, &wfds, &efds);
- if (happened)
- {
- pfd[i].revents = happened;
- rc++;
- }
- }
+ {
+ pfd[i].revents = (pfd[i].fd < 0
+ ? 0
+ : compute_revents (pfd[i].fd, pfd[i].events,
+ &rfds, &wfds, &efds));
+ rc += pfd[i].revents != 0;
+ }
return rc;
#else
diff --git a/tests/test-poll.c b/tests/test-poll.c
index cc2b5e3..c0c48d3 100644
--- a/tests/test-poll.c
+++ b/tests/test-poll.c
@@ -166,7 +166,7 @@ poll1 (int fd, int ev, int time)
pfd.fd = fd;
pfd.events = ev;
- pfd.revents = 0;
+ pfd.revents = -1;
r = poll (&pfd, 1, time);
if (r < 0)
return r;
--
2.1.0
>From 781c36385ed07995e37ed9e5bb0b7dc0e702c969 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Fri, 20 Feb 2015 10:53:10 -0800
Subject: [PATCH 2/2] getdtablesize: port better for Android
Problem reported by Kevin Cernekee in:
http://lists.gnu.org/archive/html/bug-gnulib/2015-02/msg00112.html
* doc/glibc-functions/getdtablesize.texi (getdtablesize): Mention bug.
* lib/getdtablesize.c (getdtablesize): Don't fall back on _SC_OPEN_MAX.
Instead, just use getrlimit, taking care to avoid Cygwin bug.
dup2, fcntl: cross-compile better for Android
---
ChangeLog | 9 ++++++++-
doc/glibc-functions/getdtablesize.texi | 2 +-
lib/getdtablesize.c | 35 +++++++++++++++++-----------------
3 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 6f2118c..67ad015 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2015-02-20 Paul Eggert <egg...@cs.ucla.edu>
+ getdtablesize: port better for Android
+ Problem reported by Kevin Cernekee in:
+ http://lists.gnu.org/archive/html/bug-gnulib/2015-02/msg00112.html
+ * doc/glibc-functions/getdtablesize.texi (getdtablesize): Mention bug.
+ * lib/getdtablesize.c (getdtablesize): Don't fall back on _SC_OPEN_MAX.
+ Instead, just use getrlimit, taking care to avoid Cygwin bug.
+
poll: fixes for large fds
* lib/poll.c (poll): Don't check directly for NFD too large.
Don't rely on undefined behavior in FD_SET when an arg exceeds
@@ -15,7 +22,7 @@
2015-02-18 Paul Eggert <egg...@cs.ucla.edu>
- dup2, fcntl: cross-compiler better for Android
+ dup2, fcntl: cross-compile better for Android
Problem reported by Kevin Cernekee in:
http://lists.gnu.org/archive/html/bug-gnulib/2015-02/msg00109.html
* m4/dup2.m4 (gl_FUNC_DUP2): Don't guess no when cross-compiling
diff --git a/doc/glibc-functions/getdtablesize.texi b/doc/glibc-functions/getdtablesize.texi
index 4c2cc85..f1314f0 100644
--- a/doc/glibc-functions/getdtablesize.texi
+++ b/doc/glibc-functions/getdtablesize.texi
@@ -17,7 +17,7 @@ Android LP32.
@item
This function does not represent the true @code{RLIMIT_NOFILE} soft
limit on some platforms:
-Cygwin 1.7.25.
+Android LP32, Cygwin 1.7.25.
@end itemize
Portability problems not fixed by Gnulib:
diff --git a/lib/getdtablesize.c b/lib/getdtablesize.c
index 9fe7462..bad45f7 100644
--- a/lib/getdtablesize.c
+++ b/lib/getdtablesize.c
@@ -84,32 +84,31 @@ getdtablesize (void)
return dtablesize;
}
-#elif HAVE_GETDTABLESIZE && HAVE_DECL_GETDTABLESIZE
+#else
+# include <limits.h>
# include <sys/resource.h>
-# undef getdtablesize
+
+# ifdef __CYGWIN__
+ /* Cygwin 1.7.25 auto-increases the RLIMIT_NOFILE soft limit until it
+ hits the compile-time constant hard limit of 3200. We might as
+ well just report the hard limit. */
+# define rlim_cur rlim_max
+# endif
int
-rpl_getdtablesize(void)
+getdtablesize (void)
{
- /* To date, this replacement is only compiled for Cygwin 1.7.25,
- which auto-increased the RLIMIT_NOFILE soft limit until it
- hits the compile-time constant hard limit of 3200. Although
- that version of cygwin supported a child process inheriting
- a smaller soft limit, the smaller limit is not enforced, so
- we might as well just report the hard limit. */
struct rlimit lim;
- if (!getrlimit (RLIMIT_NOFILE, &lim) && lim.rlim_max != RLIM_INFINITY)
- return lim.rlim_max;
- return getdtablesize ();
-}
-#elif defined _SC_OPEN_MAX
+ if (getrlimit (RLIMIT_NOFILE, &lim) == 0
+ && 0 <= lim.rlim_cur && lim.rlim_cur <= INT_MAX
+ && lim.rlim_cur != RLIM_INFINITY
+ && lim.rlim_cur != RLIM_SAVED_CUR
+ && lim.rlim_cur != RLIM_SAVED_MAX)
+ return lim.rlim_cur;
-int
-getdtablesize (void)
-{
- return sysconf (_SC_OPEN_MAX);
+ return INT_MAX;
}
#endif
--
2.1.0
