Just curious, which are all the modules need help with? I am interested in
contributing.
Thanks.
----- Original Message ----
From: John Lam (IRONRUBY) <jflam at microsoft.com>
To: IronRuby External Code Reviewers <irbrev at microsoft.com>
Cc: "ironruby-core at rubyforge.org" <ironruby-core at
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20080506/dbb3035f/attachment.html>
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
Thanks John, I will look at the #pack and #unpack along with File. ----- Original Message ---- From: John Lam (IRONRUBY) <jflam at microsoft.com> To: "ironruby-core at rubyforge.org" <ironruby-core at rubyforge.org> Sent: Tuesday, May 6, 2008 11:14:23 AM Subject: Re: [Ironruby-core] external contribution question 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 _______________________________________________ Ironruby-core mailing list Ironruby-core at rubyforge.org http://rubyforge.org/mailman/listinfo/ironruby-core -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20080506/276f7f06/attachment.html>
I''ll add byte array support to MutableString soon, meanwhile work with List<byte> or byte[] in #pack and #unpack methods. Or start with File. Maybe I can make MutableStr before you need it for pack/unpack. Tomas From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of Unnikrishnan Nair Sent: Tuesday, May 06, 2008 9:42 AM To: ironruby-core at rubyforge.org Subject: Re: [Ironruby-core] external contribution question Thanks John, I will look at the #pack and #unpack along with File. ----- Original Message ---- From: John Lam (IRONRUBY) <jflam at microsoft.com> To: "ironruby-core at rubyforge.org" <ironruby-core at rubyforge.org> Sent: Tuesday, May 6, 2008 11:14:23 AM Subject: Re: [Ironruby-core] external contribution question 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 _______________________________________________ Ironruby-core mailing list Ironruby-core at rubyforge.org<mailto:Ironruby-core at rubyforge.org> http://rubyforge.org/mailman/listinfo/ironruby-core -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20080506/f5395a00/attachment-0001.html>
I will start working on the Files method implementation. Once I complete reasonable amount of work with all tests, I will let you all know. Thanks. ----- Original Message ---- From: John Lam (IRONRUBY) <jflam at microsoft.com> To: "ironruby-core at rubyforge.org" <ironruby-core at rubyforge.org> Sent: Tuesday, May 6, 2008 11:14:23 AM Subject: Re: [Ironruby-core] external contribution question 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 _______________________________________________ Ironruby-core mailing list Ironruby-core at rubyforge.org http://rubyforge.org/mailman/listinfo/ironruby-core -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20080506/422b6462/attachment.html>