Re: New IO Native Provider Interface

[Aaron M. Renn]

Tom Tromey (address@hidden) wrote:
> Jeroen> - FileOutputStream should not request read access, so the modes should
> Jeroen> be "w" and "a"
> 
> I have a slight preference to use ints for the flags, not Strings.

I typically prefer ints as well.  However, since RandomAccessFile uses
strings for the access mode, I stayed with that approach.
 
> Jeroen> - write(byte[] buf, long offset, long length) should not use longs for
> Jeroen> offset and length
> Jeroen> - nativeReadByte shouldn't return a long
> 
> Agreed.  In general the argument and return types should match what
> the higher-level API is capable of.  So for instance the byte argument
> to nativeWriteByte should just be an `int', as that is what is used in
> the other layers.  Then we don't need casts anywhere.

I'm happy to make everything an int.  I think we only need a few things
like nativeFd to be longs in order to assure 64bit compatibility.  What do 
you think?
 
> I don't understand why there is a nativeValid method and checks for
> nativeFd==-1.  Shouldn't these be the same?  If not, when will they be
> different?  I think I see `nativeValid' as testing `is this
> FileDescriptor valid?'.

The Classpath implementation does a random operation on the file descriptor
to see if it works as an extra test.  It probably isn't really necessary.
Can we always guarantee that we'll never have to do a native call to
verify the validity of a file descriptor?  Can we just rely on the -1
always?

I did put -1 checks and lots of other validity checks into the Java method
wrappering ever native operation.  No need to drop to native to do checks
that are just going to fail.

> In close(), if nativeClose throws an exception, won't the
> FileDescriptor be left in an inconsistent state?

I'll take a look.  I think we should make sure that the file is marked
closed no matter what.  (What else would we do?  Leave it open and let
someone try to close it again?)_

-- 
Aaron M. Renn (address@hidden) http://www.urbanophile.com/arenn/