Hi Daniel,
On 29/04/2019, 11:34, Daniel Fuchs wrote:
Hi Michael,
I'm too new to networking libs to actually review
this change. However I have eyeballed it and it looks
like a very nice simplification and cleanup!
I didn't see anything that looked wrong.
Two thing though:
java/net/Socket.java: (and at multiple other places in this file)
1615 // Before 1.3 Sockets were always connected during creation
I believe that with your change this comment is now obsolete.
Maybe it should be altered / removed?
AbsctractSocketImpl/PlainSocketImpl:
It's a bit surprising that createSocket has now acquired
an isServer boolean, as it makes it look as if a value
different than that given to the constructor could be
passed in. Aren't the PlainSocketImpl subclasses all able
to access this `isServer` field?
I understand that the unix impl needs to pass the value
of that field to the native, but maybe it could simply
have a one arg java createSocket method that calls
the underlying two args native impl?
I think that's a fair point. Actually, the 'isServer' field was added
after other changes had already been made. But, particularly since
the field/parameter is not used on windows then it could be
refactored such that it doesn't appear in Windows and the abstract
socketCreate() method would not need it either.
Thanks,
Michael.
