Re: kern/46464: lib/librumphijack/t_tcpip:ssh test case randomly fails

[Antti Kantee <pooka%NetBSD.org@localhost>]

The following reply was made to PR kern/46464; it has been noted by GNATS.

From: Antti Kantee <pooka%NetBSD.org@localhost>
To: Andreas Gustafsson <gson%gson.org@localhost>
Cc: gnats-bugs%NetBSD.org@localhost
Subject: Re: kern/46464: lib/librumphijack/t_tcpip:ssh test case randomly
 fails
Date: Sun, 20 Jul 2014 23:17:33 +0000

 This is a multi-part message in MIME format.
 --------------010505010900080406060102
 Content-Type: text/plain; charset=ISO-8859-1; format=flowed
 Content-Transfer-Encoding: 7bit
 
 On 20/07/14 13:30, Andreas Gustafsson wrote:
 > Antti Kantee wrote:
 >> Can NetBSD's poll() return with POLLIN set in revents, but not
 >> reflecting that in the return value?  Is that legal behaviour?
 >>
 >> Or is there something else fishy going on?
 >
 > I added some debug logging to hostpoll(), after the op_pollts() call:
 >
 >      if ((parg->pfds[parg->nfds-1].revents & POLLIN) && !(rv > 0)) {
 >              fprintf(stderr, "this should not happen; rv = %d, errno = 
 > %d\n", (int)rv, errno);
 >      }
 >
 > and got the output:
 >
 >      this should not happen; rv = -1, errno = 4
 >
 > errno 4 is EINTR.  As far as I can see, POSIX is silent on what
 > revents is allowed to contain on an error return.
 
 Ok, so how about the attached patch where we trust the return value over 
 the contents of the vector.
 
 --------------010505010900080406060102
 Content-Type: text/x-patch;
  name="hijack.diff"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
  filename="hijack.diff"
 
 ? hijack.diff
 Index: hijack.c
 ===================================================================
 RCS file: /cvsroot/src/lib/librumphijack/hijack.c,v
 retrieving revision 1.108
 diff -u -r1.108 hijack.c
 --- hijack.c   19 Jul 2014 14:14:21 -0000      1.108
 +++ hijack.c   20 Jul 2014 23:13:33 -0000
 @@ -2069,12 +2069,10 @@
                errno_host = parg.errnum;
  
                /* strip cross-thread notification from real results */
 -              if (pfd_host[nfds].revents & POLLIN) {
 -                      assert(rv_host > 0);
 +              if (rv_host > 0 && pfd_host[nfds].revents & POLLIN) {
                        rv_host--;
                }
 -              if (pfd_rump[nfds].revents & POLLIN) {
 -                      assert(rv_rump > 0);
 +              if (rv_rump > 0 && pfd_rump[nfds].revents & POLLIN) {
                        rv_rump--;
                }
  
 
 --------------010505010900080406060102--