You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Joe Orton <jo...@redhat.com> on 2007/02/20 09:44:57 UTC
Re: svn commit: r509038 - in /apr/apr/trunk: CHANGES include/apr_poll.h poll/unix/epoll.c poll/unix/kqueue.c test/testpoll.c
On Mon, Feb 19, 2007 at 12:31:02AM -0000, Paul Querna wrote:
> Author: pquerna
> Date: Sun Feb 18 16:31:01 2007
> New Revision: 509038
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=509038
> Log:
> Add the apr_pollcb API.
>
> This mostly sitting in the pollcb-dev branch for several months at:
> <https://svn.apache.org/repos/asf/apr/apr/branches/pollcb-dev/>
>
> The version in this commit has small improvements, and an KQueue backend.
>
> This will likely break trunk for operating systems were we only have
> select|poll, so we will need to add some autoconf foo to define a
> HAVE_APR_POLLCB unless someone can implement apr_pollcb for all
> platforms.
And yea, verily, the build broke! Does this really need autoconf help?
It just needs some ENOTIMPL stubs #if !defined(HAVE_KQUEUE) &&
!defined(HAVE_etc etc somewhere?
Also a significant lack of API documentation here!
joe
Re: svn commit: r509038 - in /apr/apr/trunk: CHANGES include/apr_poll.h
poll/unix/epoll.c poll/unix/kqueue.c test/testpoll.c
Posted by Paul Querna <ch...@force-elite.com>.
Joe Orton wrote:
> On Tue, Feb 20, 2007 at 09:02:41AM -0800, Paul Querna wrote:
>> Paul Querna wrote:
...
> - the size parameter to _create() is documented to constrain the
> number of descriptors "returned" by _poll(), but that function
> doesn't really return descriptors.
Righto, it is the maximum number of events that can be handled by a
single call to _poll(); It is not actually the maximum number of
descriptors.
> - relatedly; _add() does not enforce the number of descriptors added
> to "nalloc", nor dynamically grow the pollset; _poll() will crash
> and burn if the user adds too many descriptors - is that
> intentional?
Nope, There is no need to dynamically grow the pollset -- The internals
of the apr_pollcb_t structure do not track the descriptors. Both KQueue
and EPoll require an array of (kevent|epoll_event) structures to be
passed in. In a single call to the OS event function, they will only
return up to nalloc events at a time -- There could be more waiting, or
only 1 returned.
Re: svn commit: r509038 - in /apr/apr/trunk: CHANGES include/apr_poll.h poll/unix/epoll.c poll/unix/kqueue.c test/testpoll.c
Posted by Joe Orton <jo...@redhat.com>.
On Tue, Feb 20, 2007 at 09:02:41AM -0800, Paul Querna wrote:
> Paul Querna wrote:
> > Sure, I can add stubs with ENOTIMPL if thats the way people want to go :-)
>
> r509637 adds stubs for poll and select.
>
> >> Also a significant lack of API documentation here!
> >
> > Yup, Its on my todo to add the API docs.... but at least there was one
> > test case, right?
It made my day.
> r509647 adds doxygen comments and docs for the pollcb functions.
> (Not really happy with all of the remarks, need to write something
> explaining why you would want to use it over apr_pollset*).
Thanks Paul. A few questions:
- the "flags" argument to _create() must currently be zero since no
flags are supported, correct? (and the APR_POLLSET_* flags are not
relevant)
- the size parameter to _create() is documented to constrain the
number of descriptors "returned" by _poll(), but that function
doesn't really return descriptors.
- relatedly; _add() does not enforce the number of descriptors added
to "nalloc", nor dynamically grow the pollset; _poll() will crash
and burn if the user adds too many descriptors - is that
intentional?
joe
Re: svn commit: r509038 - in /apr/apr/trunk: CHANGES include/apr_poll.h
poll/unix/epoll.c poll/unix/kqueue.c test/testpoll.c
Posted by Paul Querna <ch...@force-elite.com>.
Paul Querna wrote:
> Joe Orton wrote:
>> On Mon, Feb 19, 2007 at 12:31:02AM -0000, Paul Querna wrote:
>>> Author: pquerna
>>> Date: Sun Feb 18 16:31:01 2007
>>> New Revision: 509038
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=509038
>>> Log:
>>> Add the apr_pollcb API.
>>>
>>> This mostly sitting in the pollcb-dev branch for several months at:
>>> <https://svn.apache.org/repos/asf/apr/apr/branches/pollcb-dev/>
>>>
>>> The version in this commit has small improvements, and an KQueue backend.
>>>
>>> This will likely break trunk for operating systems were we only have
>>> select|poll, so we will need to add some autoconf foo to define a
>>> HAVE_APR_POLLCB unless someone can implement apr_pollcb for all
>>> platforms.
>> And yea, verily, the build broke! Does this really need autoconf help?
>> It just needs some ENOTIMPL stubs #if !defined(HAVE_KQUEUE) &&
>> !defined(HAVE_etc etc somewhere?
>
> Sure, I can add stubs with ENOTIMPL if thats the way people want to go :-)
r509637 adds stubs for poll and select.
>> Also a significant lack of API documentation here!
>
> Yup, Its on my todo to add the API docs.... but at least there was one
> test case, right?
r509647 adds doxygen comments and docs for the pollcb functions.
(Not really happy with all of the remarks, need to write something
explaining why you would want to use it over apr_pollset*).
-Paul
Re: svn commit: r509038 - in /apr/apr/trunk: CHANGES include/apr_poll.h
poll/unix/epoll.c poll/unix/kqueue.c test/testpoll.c
Posted by Paul Querna <ch...@force-elite.com>.
Joe Orton wrote:
> On Mon, Feb 19, 2007 at 12:31:02AM -0000, Paul Querna wrote:
>> Author: pquerna
>> Date: Sun Feb 18 16:31:01 2007
>> New Revision: 509038
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=509038
>> Log:
>> Add the apr_pollcb API.
>>
>> This mostly sitting in the pollcb-dev branch for several months at:
>> <https://svn.apache.org/repos/asf/apr/apr/branches/pollcb-dev/>
>>
>> The version in this commit has small improvements, and an KQueue backend.
>>
>> This will likely break trunk for operating systems were we only have
>> select|poll, so we will need to add some autoconf foo to define a
>> HAVE_APR_POLLCB unless someone can implement apr_pollcb for all
>> platforms.
>
> And yea, verily, the build broke! Does this really need autoconf help?
> It just needs some ENOTIMPL stubs #if !defined(HAVE_KQUEUE) &&
> !defined(HAVE_etc etc somewhere?
Sure, I can add stubs with ENOTIMPL if thats the way people want to go :-)
> Also a significant lack of API documentation here!
Yup, Its on my todo to add the API docs.... but at least there was one
test case, right?
-Paul