Re: Socket timeout patch

[Nic Ferrier <nferrier at tf1 dot tapsellferrier dot co dot uk>]

>>Is this part of the GCJ code standard? Does it have to change before
>>the patch is accepted?
>>
>It is the convention followed throughout libjava and classpath. It would 
>be nice to keep it consistent.

Harumph. Ok.


>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 don't think Sun's does... going by what I've seen from dumpclass
output.


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

Except that I find it confusing, and therefore someone else
might. Also, this seems like a GCJ implementation thing... is there
not a possibility it might change?


>I seem to remember seeing "connection reset by peer" exceptions when I 
>was last working on a sockets application with gcj. Perhaps I am 
>mistaken or gcj was incorrect to throw it, but the code does seem 
>inconsistent - it throws for ECONNRESET in the read() case but not in 
>the write() case.

I don't think Sun's java does send a "connection reset by peer"
exception, I can't remember. It took me some time to get to the bottom
of what Sun's implementation was actually doing, it didn't seem to be
a very good implementation.

Write may well be different, I didn't really spend a lot of time with
it, I just imported it from the io library.

You can't have timeouts on writes, it's not allowed by the spec (until
1.4).


>Just because fnum == 0 is a "normal" condition which indicates close() 
>has been called, while an EBADF in other cases would suggest some other 
>error condition/bug in our code and it would be nice to throw in that
>case.

Hmm... I really can't rememeber, I think it's there for a reason
(other than the 0 case).


>I'm sorry to be so pedantic, but it is important to try to keep the code 
>consistent and clean in order to preserve its long-term
>maintainability.

I agree... I don't think this needs to be done at the expense of
functionality though.

This patch could be in the main branch by now if it weren't for people
picking holes in various style things. This is the worst project I
know for that.

The reason it's so annoying is that all the style changes mean
constant re-testing with the main branch (witness the problem I had
with Tom's gij -jar introduced bug). If I could check the damn thing
in then I could improve the style later.


Nic