commentary, consistency, simplification, and minor fixes

impl_pollset_create():
. return the actual port_create() failure instead of APR_ENOMEM

impl_pollset_add():
. return the actual port_associate() failure instead of APR_ENOMEM

impl_pollset_poll():
. catch port_associate() failures
. don't report returned events to caller unless something popped
  besides the wakeup pipe

impl_pollcb_poll():
. fix incorrect mapping of EINTR onto APR_TIMEUP
. don't hide interesting error codes behind APR_EGENERAL

generally:
. axe redundant APR_RING_EMPTY() invocations
. don't check for EINTR explicitly, as it is handled appropriately
  by the apr_get_netos_error() invocation (IOW, EINTR == APR_EINTR here)



git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@749049 13f79535-47bb-0310-9956-ffa450edef68

1 files changed, 54 insertions, 44 deletions

diff --git a/poll/unix/port.c b/poll/unix/port.c
index c772355d1..27c99f207 100644
--- a/poll/unix/port.c
+++ b/poll/unix/port.c
@@ -36,6 +36,9 @@ static apr_int16_t get_event(apr_int16_t event)
         rv |= POLLPRI;
     if (event & APR_POLLOUT)
         rv |= POLLOUT;
+    /* TODO: Confirm that the set of return-only events is the same as with 
+     * poll(), and axe these checks:
+     */
     if (event & APR_POLLERR)
         rv |= POLLERR;
     if (event & APR_POLLHUP)
@@ -78,6 +81,9 @@ struct apr_pollset_private_t
 #endif
     /* A ring containing all of the pollfd_t that are active */
     APR_RING_HEAD(pfd_query_ring_t, pfd_elem_t) query_ring;
+    /* A ring containing the pollfd_t that will be added on the
+     * next call to apr_pollset_poll().
+     */
     APR_RING_HEAD(pfd_add_ring_t, pfd_elem_t) add_ring;
     /* A ring of pollfd_t that have been used, and then _remove'd */
     APR_RING_HEAD(pfd_free_ring_t, pfd_elem_t) free_ring;
@@ -123,7 +129,7 @@ static apr_status_t impl_pollset_create(apr_pollset_t *pollset,
 
     if (pollset->p->port_fd < 0) {
         pollset->p = NULL;
-        return APR_ENOMEM;
+        return apr_get_netos_error();
     }
 
     {
@@ -174,12 +180,15 @@ static apr_status_t impl_pollset_add(apr_pollset_t *pollset,
         fd = descriptor->desc.f->filedes;
     }
 
+    /* If another thread is polling, notify the kernel immediately; otherwise,
+     * wait until the next call to apr_pollset_poll().
+     */
     if (apr_atomic_read32(&pollset->p->waiting)) {
         res = port_associate(pollset->p->port_fd, PORT_SOURCE_FD, fd, 
                              get_event(descriptor->reqevents), (void *)elem);
 
         if (res < 0) {
-            rv = APR_ENOMEM;
+            rv = apr_get_netos_error();
             APR_RING_INSERT_TAIL(&(pollset->p->free_ring), elem, pfd_elem_t, link);
         }
         else {
@@ -222,39 +231,35 @@ static apr_status_t impl_pollset_remove(apr_pollset_t *pollset,
         rv = APR_NOTFOUND;
     }
 
-    if (!APR_RING_EMPTY(&(pollset->p->query_ring), pfd_elem_t, link)) {
-        for (ep = APR_RING_FIRST(&(pollset->p->query_ring));
-             ep != APR_RING_SENTINEL(&(pollset->p->query_ring),
-                                     pfd_elem_t, link);
-             ep = APR_RING_NEXT(ep, link)) {
-
-            if (descriptor->desc.s == ep->pfd.desc.s) {
-                APR_RING_REMOVE(ep, link);
-                APR_RING_INSERT_TAIL(&(pollset->p->dead_ring),
-                                     ep, pfd_elem_t, link);
-                if (ENOENT == err) {
-                    rv = APR_SUCCESS;
-                }
-                break;
+    for (ep = APR_RING_FIRST(&(pollset->p->query_ring));
+         ep != APR_RING_SENTINEL(&(pollset->p->query_ring),
+                                 pfd_elem_t, link);
+         ep = APR_RING_NEXT(ep, link)) {
+
+        if (descriptor->desc.s == ep->pfd.desc.s) {
+            APR_RING_REMOVE(ep, link);
+            APR_RING_INSERT_TAIL(&(pollset->p->dead_ring),
+                                 ep, pfd_elem_t, link);
+            if (ENOENT == err) {
+                rv = APR_SUCCESS;
             }
+            break;
         }
     }
 
-    if (!APR_RING_EMPTY(&(pollset->p->add_ring), pfd_elem_t, link)) {
-        for (ep = APR_RING_FIRST(&(pollset->p->add_ring));
-             ep != APR_RING_SENTINEL(&(pollset->p->add_ring),
-                                     pfd_elem_t, link);
-             ep = APR_RING_NEXT(ep, link)) {
-
-            if (descriptor->desc.s == ep->pfd.desc.s) {
-                APR_RING_REMOVE(ep, link);
-                APR_RING_INSERT_TAIL(&(pollset->p->dead_ring),
-                                     ep, pfd_elem_t, link);
-                if (ENOENT == err) {
-                    rv = APR_SUCCESS;
-                }
-                break;
+    for (ep = APR_RING_FIRST(&(pollset->p->add_ring));
+         ep != APR_RING_SENTINEL(&(pollset->p->add_ring),
+                                 pfd_elem_t, link);
+         ep = APR_RING_NEXT(ep, link)) {
+
+        if (descriptor->desc.s == ep->pfd.desc.s) {
+            APR_RING_REMOVE(ep, link);
+            APR_RING_INSERT_TAIL(&(pollset->p->dead_ring),
+                                 ep, pfd_elem_t, link);
+            if (ENOENT == err) {
+                rv = APR_SUCCESS;
             }
+            break;
         }
     }
 
@@ -302,15 +307,23 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset,
             fd = ep->pfd.desc.f->filedes;
         }
 
-        port_associate(pollset->p->port_fd, PORT_SOURCE_FD, 
-                           fd, get_event(ep->pfd.reqevents), ep);
+        ret = port_associate(pollset->p->port_fd, PORT_SOURCE_FD, 
+                             fd, get_event(ep->pfd.reqevents), ep);
+        if (ret < 0) {
+            rv = apr_get_netos_error();
+            break;
+        }
 
         APR_RING_INSERT_TAIL(&(pollset->p->query_ring), ep, pfd_elem_t, link);
-
     }
 
     pollset_unlock_rings();
 
+    if (rv != APR_SUCCESS) {
+        apr_atomic_dec32(&pollset->p->waiting);
+        return rv;
+    }
+
     ret = port_getn(pollset->p->port_fd, pollset->p->port_set, pollset->nalloc,
                     &nget, tvptr);
 
@@ -321,10 +334,7 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset,
 
     if (ret == -1) {
         (*num) = 0;
-        if (errno == EINTR) {
-            rv = APR_EINTR;
-        }
-        else if (errno == ETIME) {
+        if (errno == ETIME) {
             rv = APR_TIMEUP;
         }
         else {
@@ -360,17 +370,17 @@ static apr_status_t impl_pollset_poll(apr_pollset_t *pollset,
             }
         }
         pollset_unlock_rings();
-        if ((*num = j))
+        if ((*num = j)) { /* any event besides wakeup pipe? */
             rv = APR_SUCCESS;
-        if (descriptors) {
-            *descriptors = pollset->p->result_set;
+            if (descriptors) {
+                *descriptors = pollset->p->result_set;
+            }
         }
     }
 
-
     pollset_lock_rings();
 
-    /* Shift all PFDs in the Dead Ring to be Free Ring */
+    /* Shift all PFDs in the Dead Ring to the Free Ring */
     APR_RING_CONCAT(&(pollset->p->free_ring), &(pollset->p->dead_ring), pfd_elem_t, link);
 
     pollset_unlock_rings();
@@ -491,11 +501,11 @@ static apr_status_t impl_pollcb_poll(apr_pollcb_t *pollcb,
                     &nget, tvptr);
 
     if (ret == -1) {
-        if (errno == ETIME || errno == EINTR) {
+        if (errno == ETIME) {
             rv = APR_TIMEUP;
         }
         else {
-            rv = APR_EGENERAL;
+            rv = apr_get_netos_error();
         }
     }
     else if (nget == 0) {