Ruby Forum IronRuby > external contribution question

Just curious, which are all the modules need help with? I am interested 
in contributing.
Thanks.

----- Original Message ----
From: John Lam (IRONRUBY) <jflam@microsoft.com>
To: IronRuby External Code Reviewers <irbrev@microsoft.com>
Cc: "ironruby-core@rubyforge.org" <ironruby-core@rubyforge.org>
Sent: Monday, May 5, 2008 12:07:13 PM
Subject: [Ironruby-core] Code Review: socket2

tfpt review /shelveset:socket2;REDMOND\jflam

Overall - looks good! We really want to turn on tests for this stuff. 
Jim's looking at getting the latest rubinius specs running so we can 
deprecate our current spec snapshot soon - hopefully this week.

A few minor nits below:

Seems like you have a couple of redundant Protocols.CastToString calls 
here - domain is *always* a MutableString based on your ctor ...

        private static Socket CreateSocket(CodeContext/*!*/ context, 
MutableString/*!*/ domain, MutableString/*!*/ type, object/*Numeric*/ 
protocol) {
            RubyExecutionContext ec = 
RubyUtils.GetExecutionContext(context);
            RubyClass rubySocketClass = 
ec.GetClass(typeof(Ruby.StandardLibrary.RubySocket));

            AddressFamily addressFamily = 
(AddressFamily)RubyUtils.GetConstant(context, rubySocketClass, 
SymbolTable.StringToId(Protocols.CastToString(context, domain)), true);
            ProtocolType protocolType = 
(ProtocolType)(Protocols.CastToFixnum(context, protocol));
            SocketType socketType = 
(SocketType)RubyUtils.GetConstant(context, rubySocketClass, 
SymbolTable.StringToId(Protocols.CastToString(context, type)), true);
            return new Socket(addressFamily, socketType, protocolType);
        }

Also ... Socket should be Socket/*!*/ since there is only one code path 
out of this method barring exceptions. I've fixed these minor things in 
my copy.

Following this analysis, this also leads to _socket in 
RubyBasicSocket:23 being a /*!*/ as well.

BTW, I really like the /*Numeric*/ annotations that you've been putting 
in your code. This could lead us to doing something smarter in the 
future in the binder like adding a parameter attribute [Numeric] to 
coerce the binder into doing some smarter things with the conversions.

Thinking aloud about this method - would it make more sense to have 
MutableString and int overloads that delegate to helpers rather than 
dropping everything into a single method like this one here?

        internal static MutableString 
ConvertToHostString(CodeContext/*!*/ context, object hostname) {
            if (hostname == null) {
                return null;
            }
            if (hostname is MutableString) {
                MutableString strHostname = (MutableString)hostname;
                // Special cases
                if (strHostname == "" ) {
                    strHostname = new MutableString("0.0.0.0");
                } else if (strHostname == "<broadcast>") {
                    strHostname = new MutableString("255.255.255.255");
                }
                return strHostname;
            }
            int iHostname;
            if (Protocols.IntegerAsFixnum(hostname, out iHostname)) {
                // 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;
                }
                return new MutableString(new 
System.Net.IPAddress(bytes).ToString());
            }
            return Protocols.CastToString(context, hostname);
        }

Some other points - seems some of the code below could be delegated to 
the Protocols.CheckSafeLevel() helpers that you added. BTW, we should 
probably spend some time and make a call about what we're going to do 
about all of the safe level stuff. Arguably this stuff isn't necessary 
due to CLR safety (especially in Silverlight contexts).

        internal static void CheckSecurity(CodeContext/*!*/ context, 
object self, string message) {
            RubyExecutionContext ec = 
RubyUtils.GetExecutionContext(context);
            if (ec.CurrentSafeLevel >= 4 && ec.IsObjectTainted(self)) {
                throw RubyExceptions.CreateSecurityError("Insecure: " + 
message);
            }
        }

Thanks,
-John

Unnikrishnan Nair:

> Just curious, which are all the modules need help with? I am
> interested in contributing.
> Thanks.

There are a lot of methods in File that need to be implemented. That 
should be a reasonable place to start. We have some bugs in Dir#glob 
that could use some attention too.

If you want a good self-contained project that will take some time, take 
a look at implementing #pack and #unpack.

Thanks,
-John