[Ironruby-core] Code Review: sockets-1

John Lam (IRONRUBY) jflam at microsoft.com
Tue May 13 14:33:19 EDT 2008


tfpt review /shelveset:sockets-1;REDMOND\jflam

I've cleaned up Peter Bacon Darwin's socket patch from last week now that we have our new MutableString implementation in. I'm running a test only job through SNAP right now.

There are a few nits that I have with some new conversion protocols, some missing overloads etc. in this shelveset, but I'd rather see this go through sooner rather than later.

1. Many methods and constructors don't have the strongly-typed overloads. Here's one in TCPSocket.cs:

[RubyConstructor]
public static TCPSocket/*!*/ CreateTCPSocket(CodeContext/*!*/ context, object remoteHost, object remotePort) {

This is not critical since it will pass the tests correctly, but we really should resolve these things at some point. Not a high priority now (this is vastly preferable to having the strongly typed overload only, and missing the object overload) since we're in a bit of a time crunch.

2. Not sure what the status of the implementation is - but we can't create instances of Socket today since there isn't a ctor defined for it.

3. A large # of PortNames are statically created at start-up (in BasicSocket.cs). Would be nice to change "tcp"/"udp" to an enum since there are only 2 values.

4. ConvertToPortNum: consider a faster search than linear search for PortName. Also, the conversion protocol at the start (port is int) looks a bit strange?

5. // Ruby uses Little Endian whereas .NET uses Big Endian IP values
                byte[] bytes = new byte[4];
                for (int i = 3; i >= 0; --i) {
                    bytes[i] = (byte)(iHostname & 0xff);
                    iHostname >>= 8;
                }

Is this code platform-independent for MRI as well?

Thanks,
-John


-------------- next part --------------
A non-text attachment was scrubbed...
Name: sockets-1.diff
Type: application/octet-stream
Size: 163281 bytes
Desc: sockets-1.diff
URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20080513/9b02178a/attachment-0001.obj>


More information about the Ironruby-core mailing list