Re: Patch: tcp sockets: removing code duplication

[Andrew Haley <aph at redhat dot com>]

Michael Koch writes:
 > Am Mittwoch, 17. Mdrz 2004 04:30 schrieb Tom Tromey:
 > > >>>>> "Michael" == Michael Koch <konqueror@gmx.de> writes:
 > >
 > > Michael> I have redone the patch. Can you take anohter look at it ?
 > >
 > > This looks reasonable to me with one minor change...
 > >
 > > Michael> +void write_helper (jint native_fd, jbyte *bytes, jint len);
 > >
 > > This and read_helper should be `static'.
 > 
 > I commited the revised patch attached.

This patch introduces random EOF exceptions while trying to read
objects from a stream.

 >  // Read a single byte from the socket.
 >  jint
 >  gnu::java::net::PlainSocketImpl$SocketInputStream::read(void)
 >  {
 > -  jbyte b;
 > -  jint timeout = this$0->timeout;
 > -  jint native_fd = this$0->native_fd;
 > -
 > -  // Do timeouts via select.
 > -  if (timeout > 0 && native_fd >= 0 && native_fd < FD_SETSIZE)
 > -    {
 > -      // Create the file descriptor set.
 > -      fd_set read_fds;
 > -      FD_ZERO (&read_fds);
 > -      FD_SET (native_fd,&read_fds);
 > -      // Create the timeout struct based on our internal timeout value.
 > -      struct timeval timeout_value;
 > -      timeout_value.tv_sec = timeout / 1000;
 > -      timeout_value.tv_usec = (timeout % 1000) * 1000;
 > -      // Select on the fds.
 > -      int sel_retval =
 > -        _Jv_select (native_fd + 1, &read_fds, NULL, NULL, &timeout_value);
 > -      // If select returns 0 we've waited without getting data...
 > -      // that means we've timed out.
 > -      if (sel_retval == 0)
 > -        throw new ::java::net::SocketTimeoutException
 > -          (JvNewStringUTF ("Read timed out") );
 > -      // If select returns ok we know we either got signalled or read some data...
 > -      // either way we need to try to read.
 > -    }
 > -
 > -  int r = _Jv_read (native_fd, &b, 1);
 > +  jbyte data;
 >  
 > -  if (r == 0)
 > -    return -1;
 > +  if (read_helper (this$0->native_fd, this$0->timeout, &data, 1) == 1)
 > +    return data;
 >  
 > -  if (::java::lang::Thread::interrupted())
 > -    {
 > -      ::java::io::InterruptedIOException *iioe =
 > -        new ::java::io::InterruptedIOException
 > -        (JvNewStringUTF("Read interrupted"));
 > -      iioe->bytesTransferred = r == -1 ? 0 : r;
 > -      throw iioe;
 > -    }
 > -  else if (r == -1)
 > -    {
 > -      // Some errors cause us to return end of stream...
 > -      if (errno == ENOTCONN)
 >          return -1;
 > -
 > -      // Other errors need to be signalled.
 > -      throw new ::java::io::IOException (JvNewStringUTF (strerror (errno)));
 > -    }
 > -
 > -  return b & 0xFF;
 >  }

We should not remove this "& 0xFF".

This patch renders DataInputStream totally broken, because negative
values signal EOF.

Andrew.