Re: Patch: tcp sockets: removing code duplication

[Michael Koch <konqueror at gmx dot de>]

Am Montag, 19. April 2004 17:22 schrieb Andrew Haley:
> 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.


Does the attached patch fixes this for you ? Okay for trunk ?


Michael


2004-04-21  Michael Koch  <konqueror@gmx.de>

	* gnu/java/net/natPlainSocketImplPosix.cc
	(SocketInputStream::read): Make sure returned data is a byte value.

Index: gnu/java/net/natPlainSocketImplPosix.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/gnu/java/net/natPlainSocketImplPosix.cc,v
retrieving revision 1.7
diff -u -r1.7 natPlainSocketImplPosix.cc
--- gnu/java/net/natPlainSocketImplPosix.cc	17 Mar 2004 08:09:32 -0000	1.7
+++ gnu/java/net/natPlainSocketImplPosix.cc	21 Apr 2004 12:14:51 -0000
@@ -380,7 +380,7 @@
   jbyte data;
 
   if (read_helper (this$0->native_fd, this$0->timeout, &data, 1) == 1)
-    return data;
+    return data && 0xFF;
 
   return -1;
 }