apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Demetriou <c.g.demetr...@gmail.com>
Subject Re: pollset_poll broken ?
Date Tue, 22 Nov 2005 16:31:59 GMT
FWIW, if submitting bugs and/or patches to the main Apache bugzilla is
the right thing, it would be helpful if that were documented.

Right now:

http://apr.apache.org/patches.html

says to submit your patches to this mailing list.  I've tried that
(even resending the patch), and AFAICT it hasn't gotten any response.



cgd

On 11/22/05, Gerry <gerry@everythingsucks.co.uk> wrote:
>
>
> You're joking - this has been around for over a year ?????
> I did try searching MARC and bz, but didn't find any reference.
>
> What's the process for getting this fixed ?
>
>  * Submit patches to the BZ ?
>  * Bother someone in particular ?
>
> Cheers
> G
>
>
> Dror Shilo <Dror.Shilo@ericom.com> wrote:
>
> > I have also found this bug along time ago but noting was done:
> >
> > see:
> >
> > >From "Dror Shilo" <Dror.Shilo@ericom.com>
> > Subject RE: bug in poll.c of apr 0.9.4
> > Date Mon, 23 Aug 2004 15:26:42 GMT
> > I use apr-0.9.4
> >
> > if you don't have the poll function
> > this makes apr to implement apr_pollset_poll with a select call ( this how it
> > is done in windows)
> > apr_status_t  apr_pollset_poll  (apr_pollset_t  *pollset, apr_interval_time_t
> >  timeout, apr_int32_t
> > *num, const apr_pollfd_t **descriptors)
> >
> > the num parameter have to return the length of descriptors
> >
> > with the Unix poll function this done correct, but the select call returns
> > the total number
> > of socket handles that are ready and contained in the fd_set  structures
> > therefore if we have 2 socket that one have an FD_READ and the second have
> > FD_READ and FD_WRITE
> > it will return 3 , but the lenght of descriptors is only 2 , this is a bug
> > there for the fix for this bug is to add this  line at file poll.c line 622
> > (*num) = j;
> >
> > Dror Shilo
> > Ericom software
> >
> >
> >
> >
> > -----Original Message-----
> > From: Gerry [mailto:gerry@everythingsucks.co.uk]
> > Sent: Tuesday, November 22, 2005 12:22 AM
> > To: dev@apr.apache.org
> > Subject: BUG: pollset_poll broken ?
> >
> >
> > Evening Gents,
> >
> > Spotted a real oddity in apr_pollset_poll in both select.c and poll.c in the
> > current version 1.2.2 and in the versions used in apache2 20.54 and 2.1.9.
> >
> > Forgive me but I haven't check the rest as I've got stuff due today into
> > test.
> >
> > I'll take the SELECT based apr_pollset_poll as an example to illustrate...
> >
> > the apr_int32_t *num return value is given as the result from the select,
> > which
> > should be the number of signalled FD's.
> >
> >      rv = select(pollset->maxfd + 1, &readset, &writeset, &exceptset,
> >                     tvptr);
> >
> >      (*num) = rv;
> >
> > This is wrong.  The reason it's wrong is because you then itterate over all
> > of
> > the file descriptors in the original query_set to find which exist in either
> > the returned readset, writeset, or exceptset.
> >
> >      if (FD_ISSET(fd, &readset) || FD_ISSET(fd, &writeset) ||
> >                  FD_ISSET(fd, &exceptset)) {
> >
> > Only when the fd matches one in either set is the result_set updated
> > accordingly, and the integer 'j' used to track the number of results.
> >
> > Hence, in some cases (affecting me Mandrake Linux 10.0 3.3.2-6mdk) the 'num'
> > out
> > value is greater than the number of items copied into the result set.
> >
> > Now this would escape the notice of most folks unless, like me, your writing
> > a
> > socks proxy for Set Top Boxes which has a continually growing and shrinking
> > list of connections and hence a very dynamic pollset...
> >
> > RESULT: walking into an invalid entry in the array and core dumping.
> >
> >
> > The Fix is, at the end of the loop when running over the file descriptors,
> > you
> > should set (*num)=j as shown below.
> >
> > Let me re-itterate that this pattern is uesed in most/all of the other
> > apr_pollset_poll implementation, goes back to APR 0.94/Apache 2.054.
> >
> > I would (really) love to fix, test, and upload the patches but I haven't the
> > time.  Sorry gets.
> >
> >
> > Kind Regards,
> > Gerry Calderhead
> >
> >
> >
> > APR_DECLARE(apr_status_t) apr_pollset_poll(apr_pollset_t *pollset,
> >                                            apr_interval_time_t timeout,
> >                                            apr_int32_t *num,
> >                                            const apr_pollfd_t **descriptors)
> > {
> >     int rv;
> >     apr_uint32_t i, j;
> >     struct timeval tv, *tvptr;
> >     fd_set readset, writeset, exceptset;
> >
> >     if (timeout < 0) {
> >         tvptr = NULL;
> >     }
> >     else {
> >         tv.tv_sec = (long) apr_time_sec(timeout);
> >         tv.tv_usec = (long) apr_time_usec(timeout);
> >         tvptr = &tv;
> >     }
> >
> >     memcpy(&readset, &(pollset->readset), sizeof(fd_set));
> >     memcpy(&writeset, &(pollset->writeset), sizeof(fd_set));
> >     memcpy(&exceptset, &(pollset->exceptset), sizeof(fd_set));
> >
> > #ifdef NETWARE
> >     if (HAS_PIPES(pollset->set_type)) {
> >         rv = pipe_select(pollset->maxfd + 1, &readset, &writeset, &exceptset,
> >                          tvptr);
> >     }
> >     else
> > #endif
> >         rv = select(pollset->maxfd + 1, &readset, &writeset, &exceptset,
> >                     tvptr);
> >
> >     (*num) = rv;
> >     if (rv < 0) {
> >         return apr_get_netos_error();
> >     }
> >     if (rv == 0) {
> >         return APR_TIMEUP;
> >     }
> >     j = 0;
> >     for (i = 0; i < pollset->nelts; i++) {
> >         apr_os_sock_t fd;
> >         if (pollset->query_set[i].desc_type == APR_POLL_SOCKET) {
> >             fd = pollset->query_set[i].desc.s->socketdes;
> >         }
> >         else {
> > #if !APR_FILES_AS_SOCKETS
> >             return APR_EBADF;
> > #else
> >             fd = pollset->query_set[i].desc.f->filedes;
> > #endif
> >         }
> >         if (FD_ISSET(fd, &readset) || FD_ISSET(fd, &writeset) ||
> >             FD_ISSET(fd, &exceptset)) {
> >             pollset->result_set[j] = pollset->query_set[i];
> >             pollset->result_set[j].rtnevents = 0;
> >             if (FD_ISSET(fd, &readset)) {
> >                 pollset->result_set[j].rtnevents |= APR_POLLIN;
> >             }
> >             if (FD_ISSET(fd, &writeset)) {
> >                 pollset->result_set[j].rtnevents |= APR_POLLOUT;
> >             }
> >             if (FD_ISSET(fd, &exceptset)) {
> >                 pollset->result_set[j].rtnevents |= APR_POLLERR;
> >             }
> >             j++;
> >         }
> >     }
> >
> >     (*num) = j;   /* <<<<<<<<< ADDED BY GERR >>>>>>>>>>>>>>>>>>>>>
*/
> >
> >     if (descriptors)
> >         *descriptors = pollset->result_set;
> >     return APR_SUCCESS;
> > }
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
>
>
>

Mime
View raw message