I beleive that apr_get_netos_error() is a better choice. APR_EGENERAL is
certainly a poor choice. Other poll methods follow the same approach e.g
epoll.c.
Here is snip from epoll.c :
-------------------------------------------
ret = epoll_wait(pollset->epoll_fd, pollset->pollset, pollset->nalloc,
timeout);
(*num) = ret;
if (ret < 0) {
rv = apr_get_netos_error();
}
else if (ret == 0) {
rv = APR_TIMEUP;
}
-------------------------------------------
Regards,
Basant.
On Tue, Feb 12, 2008 at 10:18:45PM +0200, Lucian Adrian Grijincu wrote:
> This is the code in question.
>
> if (ret == -1) {
> (*num) = 0;
> if (errno == ETIME || errno == EINTR) {
> rv = APR_TIMEUP;
> }
> else {
> rv = APR_EGENERAL;
> }
> }
>
> I don't really like the APR_EGENERAL in the else either.
> Shouldn't this be something like:
> if (ret == -1)
> {
> (*num) = 0;
> rv = apr_get_netos_error();
> }
>
> and let apr_get_netos_error handle OS to APR errors consistent with
> other architectures (select.c, poll.c)
>
> On Feb 12, 2008 9:32 PM, Basant Kukreja <[EMAIL PROTECTED]> wrote:
> > Hi,
> > I am Basant Kukreja. I was working on apache httpd bug 42580.
> > http://issues.apache.org/bugzilla/show_bug.cgi?id=42580
> >
> > I figured out that the cause of the problem might be in APR.
> >
> > apr_pollset_poll function returns APR_TIMEUP even when errno is EINTR. The
> > caller functions e.g listener_thread (in worker.c) expects this API to
> > return
> > APR_EINTR if apr_pollset_poll fails.
> >
> > Other implementation of apr_pollset_poll (as in epoll.c) handles it
> > correctly.
> >
> > Please provide your comments. Suggested patch is attached. With this patch
> > the
> > issue 42580 seems to be fixed to me.
> >
> > Regards,
> > Basant.
> >
> >
> > -- orghttpd-2.2.6/srclib/apr/poll/unix/port.c Fri Apr 13 13:54:13 2007
> > +++ httpd-2.2.6/srclib/apr/poll/unix/port.c Mon Feb 11 14:11:56 2008
> > @@ -295,7 +295,10 @@
> >
> > if (ret == -1) {
> > (*num) = 0;
> > - if (errno == ETIME || errno == EINTR) {
> > + if (errno == EINTR) {
> > + rv = APR_EINTR;
> > + }
> > + else if (errno == ETIME) {
> > rv = APR_TIMEUP;
> > }
> > else {
> >
> >
>
>
>
> --
> Lucian
