Thank You, I have committed your patch in r439667 of trunk:
http://svn.apache.org/viewvc?view=rev&revision=439667
Marco Molteni wrote:
> Hi,
>
> first of all, thanks for providing APR :-)
>
> I think I've found a bug in the kqueue implementation of apr_pollset_poll().
> I am using FreeBSD -current with apr-1.2.7, but the SVN trunk
> version of kqueue.c has the same bug:
>
>
> 232 APR_DECLARE(apr_status_t) apr_pollset_poll(apr_pollset_t *pollset,
> 233 apr_interval_time_t
> timeout,
> 234 apr_int32_t *num,
> 235 const apr_pollfd_t
> **descriptors)
> 236 {
> 237 int ret, i;
> 238 struct timespec tv, *tvptr;
> 239 apr_status_t rv = APR_SUCCESS;
> 240
> 241 if (timeout < 0) {
> 242 tvptr = NULL;
> 243 }
> 244 else {
> 245 tv.tv_sec = (long) apr_time_sec(timeout);
> 246 tv.tv_nsec = (long) apr_time_msec(timeout);
> 247 tvptr = &tv;
> 248 }
>
>
> lines 245,246 convert from timeout (usec) to struct timespec, but line 246
> assigns milliseconds to nanoseconds, with a wrong magnitude of 10^6.
>
> Net effect is that a timeout of say 999 msec becomes a timeout of 999 nsec.
>
> Please find attached a proposed patch and a test program to expose the bug.
>
> thanks
> marco
>
>
>
>
> ------------------------------------------------------------------------
>
> --- kqueue.c.orig Fri Aug 18 16:44:49 2006
> +++ kqueue.c Fri Aug 18 16:51:06 2006
> @@ -240,13 +240,13 @@ APR_DECLARE(apr_status_t) apr_pollset_po
>
> if (timeout < 0) {
> tvptr = NULL;
> }
> else {
> tv.tv_sec = (long) apr_time_sec(timeout);
> - tv.tv_nsec = (long) apr_time_msec(timeout);
> + tv.tv_nsec = (long) apr_time_usec(timeout) * 1000;
> tvptr = &tv;
> }
>
> ret = kevent(pollset->kqueue_fd, NULL, 0, pollset->ke_set,
> pollset->nalloc,
> tvptr);
> (*num) = ret;
>
>
> ------------------------------------------------------------------------
>
> /* apr_kqueue_bug.c
> Expose a bug in the kqueue version of apr_pollset_poll().
>
> compile with:
> cc -W -Wall -Werror -o apr_kqueue_bug apr_kqueue_bug.c \
> -I/usr/local/include/apr-1 -L /usr/local/lib -lapr-1
> */
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <apr_poll.h>
>
> #define CHECK(x) do { \
> rv = x; \
> if (rv != APR_SUCCESS) { printf("%s %s", #x, errmsg(rv)); exit(1); } \
> } while (0)
>
> const char *
> errmsg(apr_status_t rv)
> {
> static char buf[80];
>
> if (rv == APR_SUCCESS) {
> return "success";
> } else {
> apr_strerror(rv, buf, sizeof buf);
> return buf;
> }
> }
>
> void mywait(apr_pollset_t *pollset, apr_time_t timeout_us)
> {
> apr_status_t rv;
> apr_time_t start_us, stop_us;
> apr_pollfd_t const *ret_pfd;
> int num_pfd = 0;
>
> start_us = apr_time_now();
> rv = apr_pollset_poll(pollset, timeout_us, &num_pfd, &ret_pfd);
> stop_us = apr_time_now();
> if (! APR_STATUS_IS_TIMEUP(rv)) {
> printf("unexpected apr_pollset_poll %s", errmsg(rv));
> }
> printf("wait: asked %lld ms, waited %lld ms\n",
> apr_time_as_msec(timeout_us), apr_time_as_msec(stop_us - start_us));
> }
>
> int main()
> {
> apr_status_t rv;
> apr_pollset_t *pollset;
> apr_pool_t *pool;
> apr_pollfd_t pfd;
> apr_socket_t *sock;
>
> CHECK(apr_initialize());
> CHECK(apr_pool_create(&pool, NULL));
> CHECK(apr_pollset_create(&pollset, 512, pool, 0));
> CHECK(apr_socket_create(&sock, APR_INET, SOCK_STREAM, APR_PROTO_TCP,
> pool));
>
> pfd.desc_type = APR_POLL_SOCKET;
> pfd.reqevents = APR_POLLIN;
> pfd.desc.s = sock;
> CHECK(apr_pollset_add(pollset, &pfd));
>
> mywait(pollset, 999 * 1000); // 999 ms => will wait 0 ms
> mywait(pollset, 1999 * 1000); // 1999 ms => will wait 1000 ms
>
> return 0;
> }
