Hi Mark, Am Mittwoch, den 07.12.2005, 13:35 +0100 schrieb Mark Wielaard: > On Fri, 2005-11-11 at 17:53 +0100, Mark Wielaard wrote: > > On Wed, 2005-11-09 at 21:16 +0000, Roman Kennke wrote: > > > Ingo has abstracted out the native code from gnu.java.net.PlainSocketImpl > > > and gnu.java.net.PlainDatagramSocketImpl into a new VM interface. This > > > allows VM implementors to provide a different implementation for the > > > native parts of these classes if they wish. Is this ok to commit as it > > > is? Do you have any suggestions/improvements to the interface? We would > > > like to have a stable VM interface for this area, so maybe it would be > > > helpful to discuss this with other VM implementors... > > > > A first comment is that for the VM reference implementation I would just > > make all methods native directly, no need to chain them to some helper > > method here. Is there a reason that only nativeLeave() is synchronized?
Ok, then lets make them native directly. This is only a habit, because in many cases it turned out to be more convenient to wrap a native method in a java method, i.e. to prepare arguments or do some additional things on the java side. It doesn't matter much here I guess. > This isn't a full review, just some additional comments in addition to > my earlier remarks after studying the libgcj versions. Maybe others > (Guilhem said Kaffe also doesn't have this part merged) have some > comments and then you can sent an updated proposal/patch. (I realize > some of the comments are just reflections on previous design mistakes, > but lets try clean them up now.) yeah. > - Do we need the boolean stream argument in > VMPlainSocketImpl.create()? I think not, datagram sockets are handled separately in VMDatagramSocketImpl AFAICS. > - VMPlainSocketImpl.bind() has as comment > **** How bind to INADDR_ANY **** > Should we make a convention for that one? For example just use null > add addr? I don't really know. I am not really familiar with the net code... > - There is a PlainSocketImpl.connect() method that takes a timeout > argument. We provide a default implementation now, but shouldn't this > version move to the VMPlainSocketImpl? I don't know if our default > implementation is actually that create, maybe you want to do the > actual timeout in a more specific way. Agreed. So we would have two connect() methods in the VMPlainSocketImpl. > - Should we provide wrappers/unwrappers for the Integer and Boolean > arguments/return values of setOption() and getOption() that call the > VM versions with primitive types? That would make the JNI code much > cleaner. Sounds reasonable. I would think that value always is some primitive type, so we don't really need Object there, right? > - PlainSocketImpl.sendUrgentData() should delegate to VMPlainSocketImpl. Agreed. > - It might makes sense to have a no argument read() and write() method > in the VM interface instead of just the byte[] versions. Also agreed. > - We should explicitly document the new constant IP_TTL we use in > PlainDatagramSocketImpl for the VM interface since it isn't defined as > SocketOption. Or should we just make a special VM interface to set/get > the TTL? I vote for documenting the constant. > - The PlainDatagramSocket multicast opertations join(Group), > leave(Group) etc, need to go through to the VM interface. Agreed. I will put together a new patch that incorporates the suggested changes. Cheers, Roman
signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil
_______________________________________________ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
