bug#11508: 24.1.50; Off-by-one error in xg_select?

[Jan Djärv]

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)
> 
> 
> 
>