[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#11508: 24.1.50; Off-by-one error in xg_select?
From: |
Jan Djärv |
Subject: |
bug#11508: 24.1.50; Off-by-one error in xg_select? |
Date: |
Sat, 19 May 2012 10:05:57 +0200 |
Hello.
I think you misunderstand how select works. Here is a snippet from the BSD man
page:
"The first nfds descriptors are checked in
each set; i.e., the descriptors from 0 through nfds-1 in the descriptor
sets are examined. (Example: If you have set two file descriptors "4"
and "17", nfds should not be "2", but rather "17 + 1" or "18".) "
Jan D.
18 maj 2012 kl. 15:13 skrev Ken Brown:
> I apologize in advance for the noise if I'm misunderstanding something,
> but it seems to me that there is an error in the calculation of the
> first argument in the call to select in xgselect.c:105. (Line numbers
> refer to the current trunk; xgselect.c was last changed in bzr revno
> 108249.)
>
> The parameter "max_fds" in xg_select, in spite of its name, is initially
> 1 higher than the maximal file descriptor in the fd_sets in the other
> parameters. If max_fds doesn't get increased in line 78 or 83, then
> line 104 does the wrong thing, causing the first argument to select in
> line 105 to be 1 higher than it should be.
>
> I think the following patch fixes this. It also renames "max_fds" to
> "fds_lim" to more accurately reflect what it represents.
>
> === modified file 'src/xgselect.c'
> --- src/xgselect.c 2012-05-16 02:22:53 +0000
> +++ src/xgselect.c 2012-05-18 12:28:27 +0000
> @@ -32,7 +32,7 @@
> static ptrdiff_t gfds_size;
>
> int
> -xg_select (int max_fds, SELECT_TYPE *rfds, SELECT_TYPE *wfds, SELECT_TYPE
> *efds,
> +xg_select (int fds_lim, SELECT_TYPE *rfds, SELECT_TYPE *wfds, SELECT_TYPE
> *efds,
> EMACS_TIME *timeout)
> {
> SELECT_TYPE all_rfds, all_wfds;
> @@ -41,10 +41,10 @@
> GMainContext *context;
> int have_wfds = wfds != NULL;
> int n_gfds = 0, our_tmo = 0, retval = 0, our_fds = 0;
> - int i, nfds, fds_lim, tmo_in_millisec;
> + int i, nfds, tmo_in_millisec;
>
> if (inhibit_window_system || !display_arg)
> - return select (max_fds, rfds, wfds, efds, timeout);
> + return select (fds_lim, rfds, wfds, efds, timeout);
>
> if (rfds) memcpy (&all_rfds, rfds, sizeof (all_rfds));
> else FD_ZERO (&all_rfds);
> @@ -75,12 +75,12 @@
> if (gfds[i].events & G_IO_IN)
> {
> FD_SET (gfds[i].fd, &all_rfds);
> - if (gfds[i].fd > max_fds) max_fds = gfds[i].fd;
> + if (gfds[i].fd >= fds_lim) fds_lim = gfds[i].fd + 1;
> }
> if (gfds[i].events & G_IO_OUT)
> {
> FD_SET (gfds[i].fd, &all_wfds);
> - if (gfds[i].fd > max_fds) max_fds = gfds[i].fd;
> + if (gfds[i].fd >= fds_lim) fds_lim = gfds[i].fd + 1;
> have_wfds = 1;
> }
> }
> @@ -101,7 +101,6 @@
> if (our_tmo) tmop = &tmo;
> }
>
> - fds_lim = max_fds + 1;
> nfds = select (fds_lim, &all_rfds, have_wfds ? &all_wfds : NULL, efds,
> tmop);
>
> if (nfds < 0)
>
>
>
>