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 --------------
A non-text attachment was scrubbed...
Name: socket2.diff
Type: application/octet-stream
Size: 160634 bytes
Desc: socket2.diff
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20080505/b064af79/attachment-0001.obj>
Hi John,
What is the process of these code reviews of external contributions? Are
you expecting to begin a dialogue, since you do ask questions throughout the
review? Or is it more of a feedback opportunity to let us know what you
have done to the code? For instance, are you expecting me to make changes
discussed below and resubmit the patch - clearly not in the case of the
first item as you have said that you changed it already on your local copy?
Cheers,
Pete
-----Original Message-----
From: ironruby-core-bounces at rubyforge.org
[mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of John Lam
(IRONRUBY)
Sent: Monday,05 May 05, 2008 18:07
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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
Peter Bacon Darwin:> What is the process of these code reviews of external contributions? > Are you expecting to begin a dialogue, since you do ask questions > throughout the review? Or is it more of a feedback opportunity to let > us know what you have done to the code? For instance, are you > expecting me to make changes discussed below and resubmit the patch - > clearly not in the case of the first item as you have said that you > changed it already on your local copy?Generally, if folks have opinions about the review either way, please let us know. Specifically on your contribution, I''m having a heck of a time getting it to compile under Silverlight. I''m going to block off some more time tomorrow to see what I can do. Sockets are only partially implemented in Silverlight, so I have to figure out the delta. Don''t worry about making changes - once I get it compiling under SL, I''ll check it in and you can modify my changes :) Thanks, -John
The .NET Socket library is a fairly thin layer that sits on top of WinSock. Clearly Silverlight would not be able to do this since WinSock is not a standard API on other OSes. Also, Silverlight is going to have additional security restrictions that would prevent much of the Socket library from work anyway. To be honest, even the full .NET Framework socket implementation does not fully support all the features required by the Ruby socket library. I have been struggling to get the Socket class working - it is not pretty. This is partly to do with the way the Ruby socket library is implemented. In particular, the Socket class provides access to low level C API methods, which differ from system to system. In the best case this results in slightly inconsistent behaviour and different errors being raised. In the worst case it is not possible to write portable Ruby code that uses the socket library, unless you restrict yourself to well defined methods. It seems reasonable that, since the Ruby Socket library is somewhat OS dependent anyway, we should agree upon a subset of the functionality to support, particularly with regards to Silverlight. Perhaps this should be based upon common usage in programs like Mongrel? I suspect that like many of these things, we may end up having to code up some stuff in straight .NET (is this possible with socket-type things?) Pete John Lam wrote:> Specifically on your contribution, I''m having a heck of a time getting itto compile under Silverlight. I''m going to block off some more time tomorrow> to see what I can do. Sockets are only partially implemented inSilverlight, so I have to figure out the delta. Don''t worry about making changes - once > I get it compiling under SL, I''ll check it in and you can modify my changes :) Thanks, -John _______________________________________________ Ironruby-core mailing list Ironruby-core at rubyforge.org http://rubyforge.org/mailman/listinfo/ironruby-core
I see that the Ruby.StandardLibrary.RubySocket is implemented in C#. Is it common to write the core in C#? Has anything been written in Ruby itself? Sorry for the newbie question... is there a FAQ for this type of situation? On May 7, 2008, at 2:19 AM, Peter Bacon Darwin wrote:> The .NET Socket library is a fairly thin layer that sits on top of > WinSock. > Clearly Silverlight would not be able to do this since WinSock is > not a > standard API on other OSes. Also, Silverlight is going to have > additional > security restrictions that would prevent much of the Socket library > from > work anyway. > > To be honest, even the full .NET Framework socket implementation > does not > fully support all the features required by the Ruby socket > library. I have > been struggling to get the Socket class working - it is not pretty. > > This is partly to do with the way the Ruby socket library is > implemented. > In particular, the Socket class provides access to low level C API > methods, > which differ from system to system. In the best case this results in > slightly inconsistent behaviour and different errors being raised. > In the > worst case it is not possible to write portable Ruby code that uses > the > socket library, unless you restrict yourself to well defined methods. > > It seems reasonable that, since the Ruby Socket library is somewhat OS > dependent anyway, we should agree upon a subset of the > functionality to > support, particularly with regards to Silverlight. Perhaps this > should be > based upon common usage in programs like Mongrel? I suspect that > like many > of these things, we may end up having to code up some stuff in > straight .NET > (is this possible with socket-type things?) > > Pete > > > John Lam wrote: > >> Specifically on your contribution, I''m having a heck of a time >> getting it > to compile under Silverlight. I''m going to block off some more time > tomorrow >> to see what I can do. Sockets are only partially implemented in > Silverlight, so I have to figure out the delta. Don''t worry about > making > changes - once > I get it compiling under SL, I''ll check it in and > you can > modify my changes :) > > Thanks, > -John > > _______________________________________________ > Ironruby-core mailing list > Ironruby-core at rubyforge.org > http://rubyforge.org/mailman/listinfo/ironruby-core > > _______________________________________________ > Ironruby-core mailing list > Ironruby-core at rubyforge.org > http://rubyforge.org/mailman/listinfo/ironruby-core
The majority if not all of the IronRuby libraries have been written in C# so far. There have been discussions about this on the mailing list before. There are a number of Ruby libraries that have been written in Ruby. Once IronRuby is fully compliant then it should be able to load these and run them straight up. Obviously stuff that cannot be written in Ruby (such as accessing hardware devices and so on) has to be written in something else. In CRuby these libraries are written in C. In IronRuby these are written in a .NET language (i.e. C#). That being said, since IronRuby provides .NET interop then it is conceivable to write a Ruby library in Ruby and bounce out to the .NET framework classes as necessary. Obviously performance issues must be taken into account. Also, there are issues discussed recently about packaging up such Ruby libraries when deploying to Silverlight. Pete -----Original Message----- From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of C.J. Adams-Collier Sent: Wednesday,07 May 07, 2008 15:10 To: ironruby-core at rubyforge.org Subject: Re: [Ironruby-core] Code Review: socket2 I see that the Ruby.StandardLibrary.RubySocket is implemented in C#. Is it common to write the core in C#? Has anything been written in Ruby itself? Sorry for the newbie question... is there a FAQ for this type of situation?
C.J. Adams-Collier:> I see that the Ruby.StandardLibrary.RubySocket is implemented in C#. > Is it common to write the core in C#? Has anything been written in > Ruby itself?Our guideline is: if it was written in C in MRI, we''re writing in C#. Thanks, -John
Peter Bacon Darwin:> The .NET Socket library is a fairly thin layer that sits on top of > WinSock. > Clearly Silverlight would not be able to do this since WinSock is not a > standard API on other OSes. Also, Silverlight is going to have > additional security restrictions that would prevent much of the Socket > library from work anyway. > > To be honest, even the full .NET Framework socket implementation does > not fully support all the features required by the Ruby socket library. > I have been struggling to get the Socket class working - it is not > pretty.Do you think it would be worthwhile to just have folks use the .NET Socket support in Silverlight and not bother having a "ruby" socket implementation? In this case I still have to figure out how to conditionally compile stuff against the same Initializer.Generated.cs - I might have to add a pre-build step to force its generation. Thanks, -John
As an example, I tried this with the zlib library initially (well not the interop, just pure ruby). The result of which is Zliby ( http://zliby.rubyforge.org/), however the performance was so abysmial it pretty much had to be rewritten in C# for it to be functional. On Wed, May 7, 2008 at 10:33 AM, Peter Bacon Darwin < bacondarwin at googlemail.com> wrote:> The majority if not all of the IronRuby libraries have been written in C# > so > far. There have been discussions about this on the mailing list before. > > There are a number of Ruby libraries that have been written in Ruby. Once > IronRuby is fully compliant then it should be able to load these and run > them straight up. > > Obviously stuff that cannot be written in Ruby (such as accessing hardware > devices and so on) has to be written in something else. In CRuby these > libraries are written in C. In IronRuby these are written in a .NET > language (i.e. C#). > > That being said, since IronRuby provides .NET interop then it is > conceivable > to write a Ruby library in Ruby and bounce out to the .NET framework > classes > as necessary. Obviously performance issues must be taken into account. > Also, there are issues discussed recently about packaging up such Ruby > libraries when deploying to Silverlight. > > Pete > > -----Original Message----- > From: ironruby-core-bounces at rubyforge.org > [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of C.J. > Adams-Collier > Sent: Wednesday,07 May 07, 2008 15:10 > To: ironruby-core at rubyforge.org > Subject: Re: [Ironruby-core] Code Review: socket2 > > I see that the Ruby.StandardLibrary.RubySocket is implemented in C#. > Is it common to write the core in C#? Has anything been written in > Ruby itself? > > Sorry for the newbie question... is there a FAQ for this type of > situation? > > > _______________________________________________ > Ironruby-core mailing list > Ironruby-core at rubyforge.org > http://rubyforge.org/mailman/listinfo/ironruby-core >-- Michael Letterle [Polymath Prokrammer] http://blog.prokrams.com -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20080507/93bfaaac/attachment-0001.html>
/ideally/ we would be able to use RUBY_PLATFORM or some such to determine if we were on silverlight or not. We could have a socket.rb that loads the functions differently depending on the platform (and throws not implements if it''s not applicable to Silverlight). On Wed, May 7, 2008 at 10:38 AM, John Lam (IRONRUBY) <jflam at microsoft.com> wrote:> Peter Bacon Darwin: > > > The .NET Socket library is a fairly thin layer that sits on top of > > WinSock. > > Clearly Silverlight would not be able to do this since WinSock is not a > > standard API on other OSes. Also, Silverlight is going to have > > additional security restrictions that would prevent much of the Socket > > library from work anyway. > > > > To be honest, even the full .NET Framework socket implementation does > > not fully support all the features required by the Ruby socket library. > > I have been struggling to get the Socket class working - it is not > > pretty. > > Do you think it would be worthwhile to just have folks use the .NET Socket > support in Silverlight and not bother having a "ruby" socket implementation? > In this case I still have to figure out how to conditionally compile stuff > against the same Initializer.Generated.cs - I might have to add a pre-build > step to force its generation. > > Thanks, > -John > > _______________________________________________ > Ironruby-core mailing list > Ironruby-core at rubyforge.org > http://rubyforge.org/mailman/listinfo/ironruby-core >-- Michael Letterle [Polymath Prokrammer] http://blog.prokrams.com -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20080507/6c28332f/attachment.html>
On Wed, 2008-05-07 at 07:38 -0700, John Lam (IRONRUBY) wrote:> Peter Bacon Darwin: > > > The .NET Socket library is a fairly thin layer that sits on top of > > WinSock. > > Clearly Silverlight would not be able to do this since WinSock is not a > > standard API on other OSes. Also, Silverlight is going to have > > additional security restrictions that would prevent much of the Socket > > library from work anyway. > > > > To be honest, even the full .NET Framework socket implementation does > > not fully support all the features required by the Ruby socket library. > > I have been struggling to get the Socket class working - it is not > > pretty. > > Do you think it would be worthwhile to just have folks use the .NET Socket support in Silverlight and not bother having a "ruby" socket implementation? In this case I still have to figure out how to conditionally compile stuff against the same Initializer.Generated.cs - I might have to add a pre-build step to force its generation. > > Thanks, > -JohnDid you say PreBuild? -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20080507/68954f72/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 189 bytes Desc: This is a digitally signed message part URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20080507/68954f72/attachment.bin>
On Wed, 2008-05-07 at 07:34 -0700, John Lam (IRONRUBY) wrote:> C.J. Adams-Collier: > > > I see that the Ruby.StandardLibrary.RubySocket is implemented in C#. > > Is it common to write the core in C#? Has anything been written in > > Ruby itself? > > Our guideline is: if it was written in C in MRI, we''re writing in C#. > > Thanks, > -JohnSounds reasonable. Any thoughts on using the ''cil'' back-end for gcc to emit IL placeholders until there are enough free tuits to implement in C#? -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20080507/15572855/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 189 bytes Desc: This is a digitally signed message part URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20080507/15572855/attachment.bin>
C.J. Adams-Collier:> Sounds reasonable. Any thoughts on using the ''cil > <http://gcc.gnu.org/projects/cli.html> '' back-end for gcc to emit IL > placeholders until there are enough free tuits to implement in C#? >I doubt that would generate verifiable CIL, so it''s a non-starter for most of the things that we''re (especially Silverlight). Thanks, -John
This seems reasonable to me but then I am not a seasoned Ruby Sockets user so I don''t know what level of support developers would expect. Certainly for a first release I don''t see why we couldn''t get away without Ruby Sockets support on Silverlight. I doubt anyone coding up a Silverlight Ruby app is going to worry that their code is portable to non-IronRuby platform. The big question is whether there are any libraries that require the Ruby Socket library that might be of use to people writing Silverlight apps. Pete -----Original Message----- From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of John Lam (IRONRUBY) Sent: Wednesday,07 May 07, 2008 15:38 To: ironruby-core at rubyforge.org Subject: Re: [Ironruby-core] Code Review: socket2 Peter Bacon Darwin:> The .NET Socket library is a fairly thin layer that sits on top of > WinSock. > Clearly Silverlight would not be able to do this since WinSock is not a > standard API on other OSes. Also, Silverlight is going to have > additional security restrictions that would prevent much of the Socket > library from work anyway. > > To be honest, even the full .NET Framework socket implementation does > not fully support all the features required by the Ruby socket library. > I have been struggling to get the Socket class working - it is not > pretty.Do you think it would be worthwhile to just have folks use the .NET Socket support in Silverlight and not bother having a "ruby" socket implementation? In this case I still have to figure out how to conditionally compile stuff against the same Initializer.Generated.cs - I might have to add a pre-build step to force its generation. Thanks, -John _______________________________________________ Ironruby-core mailing list Ironruby-core at rubyforge.org http://rubyforge.org/mailman/listinfo/ironruby-core