YA Socket timeout patch

[Bryce McKinlay]

Nic Ferrier wrote:

>Personally, I think you should follow the convention for javadoc that
>I use on the Paperclips project, because it's better and consistent
>and it looks good.
>
>I use the first line (the one with the double **) to be a very brief
>intro, as per the Sun suggestions. This means you can right very
>simple search tools to pull out doc intro's to things by scanning
>lines for **.
>

Perhaps this approach is not without merit, however it deviates from the 
convention followed elsewhere in classpath. If you want to change it, 
you'd really have to take up the issue on classpath@gnu.org.

>The doc comments that are just a single line I have not altered. It
>seems a waste to make a line for nothing.
>

I agree with you here.

>Bryce came back:
>
>>I agree that it makes the code clearer in some cases to explicitly 
>>specify it, but it does cause GCJ to generate less efficient code. 
>>Perhaps we can fix GCJ to remove/ignore the redundant assignment though. 
>>Do other bytecode compilers generate field initializer code for null and 
>>zero initializations?
>>
>
>I think we sorted this one. The value has to be initialized if you're
>going to rely on it. I wouldn't change this even if it were not true:
>not initializing goes against all my feelings about code readability
>and correctness.
>

I don't agree. In this case there is no superclass that is going to mess 
with the value, it is safe and more efficient to not explicitly 
initialize it. I'd prefer if you removed them.

>Bryce came back:
>
>>No. Only the java compiler ever generates class objects - so there is 
>>only one class object  even if the class has native methods. In this 
>>case, since the SocketInput/OutputStream classes are inner classes 
>>anyway, I see no reason to not just implement their native methods in 
>>natPlainSocketImpl.cc also.
>>
>
>I didn't like the idea anyway... I don't think it's very clear what's
>going on... So, instead of making them native I made them final. I
>think they should be final anyway.
>

OK. (btw "final" probibly wont make any difference to efficiency because 
it will be a virtual call anyway, but there is no harm in specifying it).

>+  else if (r == -1)
>+    {
>+      if (errno == ECONNRESET)
>+	throw new java::io::IOException (JvNewStringUTF (strerror (errno)));
>+      // 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;
>+}
>

Also, you still have a redundant check for errno == ECONNRESET in the 
read() methods?

If you address these issues, the patch is ok to check in. Thanks!

regards

Bryce.