apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Querna <c...@force-elite.com>
Subject Re: apr_poll* changes
Date Tue, 05 Oct 2004 02:40:23 GMT
Joe Orton wrote:
> Thanks for persevering with this.  Some review below - which of the
> implementations have you tested with this?

The attached, updated patch, has been tested on:

EPoll: None. (I tested the patch I sent earlier today with EPoll, and it 
worked.  I just don't have access to any machines with EPoll at Home.)

KQueue: NetBSD 2.0, OS X 10.3.5, FreeBSD 4.10 (minotaur)

Poll: NetBSD 2.0, Gentoo Linux, Solaris 10.b58

Select: NetBSD 2.0

kqueue.c was missing a few changes from my original patch. I had 
accidentally missed them after splitting the files.

I also switched all the #ifdef APR_HAS_THREADS to #if APR_HAS_THREADS, 
since it is always defined, but set to 0 or 1. (This fixes the patch 
FreeBSD 4.x).

I renamed pfd_elem_t to apr_pfd_elem_t.

> Quite a lot of unnecessary whitespace changes, e.g:

I believe all of these changes are in accordance with the style 

I made style 'fixes' in the new files, and did the same with what is 
left of poll.c. I guess I shouldn't treat the changed poll.c as a new file?

I can undo the style changes for poll.c if you want them in a separate 

> Lack of error handling and s,if(,if (,g
>>+#define pollset_lock_rings() \
>>+    if(pollset->flags & APR_POLLSET_THREADSAFE) \
>>+        apr_thread_mutex_lock(pollset->ring_lock);
>>+#define pollset_unlock_rings() \
>>+    if(pollset->flags & APR_POLLSET_THREADSAFE) \
>>+        apr_thread_mutex_unlock(pollset->ring_lock);

Is it possible for these functions to fail?  I am pretty sure there are 
places in httpd where the return values are not checked.  Should they 
always be checked?

My man pages say that pthread_mutex_lock will only fail with:

[EINVAL] - The value specified by mutex is invalid.
[EDEADLK] - A deadlock would occur if the thread blocked waiting for mutex.

So, either a double call to _lock() - causing deadlock - or an invalid 
mutex.  I am not sure if adding a generic 'return rv;' to the defines 
would make any difference.

-Paul Querna

View raw message