RFC: FileDescriptor patch

[Tom Tromey]

Here's a rough draft of a patch I'd like to check in.  It isn't
finished, but I'd like some comments on it first.

Basically I'd like to simplify FileDescriptor in a few ways.  I added
a new convenience constructor, removed the need for some special error
handling, changed open() flags to int constants, changed
FileDescriptor to force nativeInit to initialize the in/out/err
fields, and removed readOnly from RandomAccessFile (the idea being,
the native file descriptor should enforce this constraint).

Mostly this stuff comes from libgcj.

This probably doesn't even build yet.

Tom

Index: ChangeLog
from  Tom Tromey  <address@hidden>

        * java/io/FileInputStream.java (FileInputStream): Updated for
        changes to FileDescriptor.
        * java/io/FileOutputStream.java (FileOutputStream): Updated for
        changes to FileDescriptor.
        * java/io/RandomAccessFile.java (RandomAccessFile): Updated for
        changes to FileDescriptor.
        (readOnly): Removed; updated all users.
        * java/io/FileDescriptor.java (SET, CUR, END): Now constants.
        (READ, WRITE, APPEND, EXCL): New constants.
        (in, out, err): Require nativeInit to initialize.
        (FileDescriptor(String,int)): New constructor.
        (open): Changed argument type of `mode'.  Allow empty paths.
        Only throw runtime errors or FileNotFoundException.
        (nativeValid): Don't throw IOException.
        (valid): Simplified.
        (nativeOpen): Throw FileNotFoundException.  Changed type of
        `mode' argument.
        * native/jni/java-io/FileDescriptor.c
        (Java_java_io_FileDescriptor_nativeValid): Updated comment.
        (SET, CUR, END, READ, WRITE, APPEND, EXCL): New defines.
        (Java_java_io_FileDescriptor_nativeOpen): Changed type of mode
        argument.

Index: java/io/FileDescriptor.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/io/FileDescriptor.java,v
retrieving revision 1.14
diff -u -r1.14 FileDescriptor.java
--- java/io/FileDescriptor.java 23 Mar 2003 10:53:30 -0000 1.14
+++ java/io/FileDescriptor.java 5 Apr 2003 20:16:49 -0000
@@ -56,22 +56,20 @@
           System.loadLibrary ("javaio");
         }
 
-      // nativeInit() should override these values if appropriate
-      in = new FileDescriptor(0);
-      out = new FileDescriptor(1);
-      err = new FileDescriptor(2);
-
-      SET = 0;
-      CUR = 1;
-      END = 2;
-
       nativeInit();
     }
 
   // Use for seeking
-  static final int SET;
-  static final int CUR;
-  static final int END;
+  static final int SET = 0;
+  static final int CUR = 1;
+  static final int END = 2;
+
+  // These are mode values for open().
+  static final int READ   = 1;
+  static final int WRITE  = 2;
+  static final int APPEND = 4;
+  // EXCL is used only when making a temp file.
+  static final int EXCL   = 8;
 
   /**
    * A <code>FileDescriptor</code> representing the system standard input
@@ -111,6 +109,11 @@
     this.nativeFd = nativeFd;
   }
 
+  FileDescriptor(String path, int mode) throws FileNotFoundException
+  {
+    open(path, mode);
+  }
+
   /**
    * This method forces all data that has not yet been physically written to
    * the underlying storage medium associated with this 
@@ -140,30 +143,22 @@
     if (nativeFd == -1L)
       return(false);
 
-    try
-      {
-        return(nativeValid(nativeFd));
-      }
-    catch (IOException e)
-      {
-        return(false);
-      }
+    return nativeValid(nativeFd);
   }
 
-  void open(String path, String mode) throws IOException
+  void open(String path, int mode) throws FileNotFoundException
   {
     // We don't want fd leakage.
     if (nativeFd != -1L)
-      throw new IOException("FileDescriptor already open");
+      throw new InternalError("FileDescriptor already open");
 
-    if ((path == null) || path.equals(""))
-      throw new IllegalArgumentException("Path cannot be null");
+    // Note that it can be ok to have an empty path.
+    // FIXME: verify
+    if (path == null)
+      throw new NullPointerException("Path cannot be null");
 
-    if (!mode.equals("r") && !mode.equals("rw") && !mode.equals("rws") &&
-        !mode.equals("rwd") && !mode.equals("rwa") && !mode.equals("w") &&
-        !mode.equals("a"))
-      throw new IllegalArgumentException("Invalid mode value: " + mode);
-    // Note above implicitly checks mode for null value
+    if ((mode & (READ | WRITE)) == 0)
+      throw new InternalError("Invalid mode value: " + mode);
 
     nativeFd = nativeOpen(path, mode);
   }
@@ -332,7 +327,8 @@
    *
    * @exception IOException If an error occurs.
    */
-  private native long nativeOpen(String path, String mode) throws IOException;
+  private native long nativeOpen(String path, int mode)
+    throws FileNotFoundException;
 
   /**
    * Closes this specified file descriptor
@@ -472,10 +468,8 @@
    *
    * @return <code>true</code> if the fd is valid, <code>false</code> 
    * otherwise
-   *
-   * @exception IOException If an error occurs
    */
-  private native boolean nativeValid(long fd) throws IOException;
+  private native boolean nativeValid(long fd);
 
   /**
    * Flushes any buffered contents to disk
@@ -487,4 +481,3 @@
   private native void nativeSync(long fd) throws SyncFailedException;
 
 } // class FileDescriptor
-
Index: java/io/FileInputStream.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/io/FileInputStream.java,v
retrieving revision 1.21
diff -u -r1.21 FileInputStream.java
--- java/io/FileInputStream.java 24 Mar 2003 10:19:19 -0000 1.21
+++ java/io/FileInputStream.java 5 Apr 2003 20:16:49 -0000
@@ -83,16 +83,7 @@
     if (s != null)
       s.checkRead(name);
 
-    fd = new FileDescriptor();
- 
-    try
-      {
-        fd.open(name, "r");
-      }
-    catch(IOException e)
-      {
-        throw new FileNotFoundException(name + ": " + e.getMessage());
-      }
+    fd = new FileDescriptor(name, FileDescriptor.READ);
   }
 
   /**
Index: java/io/FileOutputStream.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/io/FileOutputStream.java,v
retrieving revision 1.21
diff -u -r1.21 FileOutputStream.java
--- java/io/FileOutputStream.java 28 Mar 2003 08:39:16 -0000 1.21
+++ java/io/FileOutputStream.java 5 Apr 2003 20:16:49 -0000
@@ -85,19 +85,10 @@
     if (s != null)
       s.checkWrite(path);
 
-    fd = new FileDescriptor();
-
-    try 
-      {
-        if (append)
-          fd.open(path, "a");
-        else
-          fd.open(path, "w");
-      }
-    catch(IOException e)
-      {
-        throw new FileNotFoundException(path + ": " + e.getMessage());
-      }
+    int flags = FileDescriptor.WRITE;
+    if (append)
+      flags |= FileDescriptor.APPEND;
+    fd = new FileDescriptor(path, flags);
   }
 
   /**
Index: java/io/RandomAccessFile.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/io/RandomAccessFile.java,v
retrieving revision 1.27
diff -u -r1.27 RandomAccessFile.java
--- java/io/RandomAccessFile.java 31 Mar 2003 09:42:24 -0000 1.27
+++ java/io/RandomAccessFile.java 5 Apr 2003 20:16:49 -0000
@@ -65,11 +65,6 @@
   
   private FileChannel ch; /* cached associated file-channel */
   
-  /**
-   * Whether or not this file is open in read only mode
-   */
-  private boolean readOnly;
-  
   // Used for DataOutput methods writing values to the underlying file
   private byte[] buf = new byte[8];
   
@@ -124,29 +119,24 @@
     if (!mode.equals("r") && !mode.equals("rw") && !mode.equals("rws") &&
         !mode.equals("rwd"))
       throw new IllegalArgumentException("Bad mode value: " + mode);
-  
+
+    int fdmode;
+    if (mode.compareTo ("r") == 0)
+      fdmode = FileDescriptor.READ;
+    else
+      fdmode = FileDescriptor.READ | FileDescriptor.WRITE;
+
     // The obligatory SecurityManager stuff
     SecurityManager s = System.getSecurityManager();
     if (s != null)
       {
         s.checkRead(fileName);
 
-        if (!mode.equals("r"))
+        if ((fdmode & FileDescriptor.WRITE) != 0)
           s.checkWrite(fileName);
       }
-  
-    if (mode.equals("r"))
-      readOnly = true;
-  
-    fd = new FileDescriptor();
-    try
-      {
-        fd.open(fileName, mode);
-      }
-    catch(IOException e)
-      {
-        throw new FileNotFoundException(e.getMessage()); 
-      }
+
+    fd = new FileDescriptor(fileName, mode);
   }
 
   /**
@@ -205,9 +195,6 @@
    */
   public void setLength(long newlen) throws IOException
   {
-    if (readOnly)
-      throw new IOException("File is open read only");
-  
     fd.setLength(newlen);
   }
 
@@ -824,9 +811,6 @@
    */
   public void write (int oneByte) throws IOException
   {
-    if (readOnly)
-      throw new IOException("File is open read only");
-  
     fd.write (oneByte);
   }
 
@@ -853,9 +837,6 @@
    */
   public void write (byte[] buffer, int offset, int len) throws IOException
   {
-    if (readOnly)
-      throw new IOException("File is open read only");
-  
     fd.write (buffer, offset, len);
   }
 
Index: native/jni/java-io/FileDescriptor.c
===================================================================
RCS file: /cvsroot/classpath/classpath/native/jni/java-io/FileDescriptor.c,v
retrieving revision 1.6
diff -u -r1.6 FileDescriptor.c
--- native/jni/java-io/FileDescriptor.c 4 Apr 2003 05:29:14 -0000 1.6
+++ native/jni/java-io/FileDescriptor.c 5 Apr 2003 20:16:50 -0000
@@ -42,7 +42,7 @@
  *
  * Aaron M. Renn (address@hidden)
  *
- * Some of this coded adoped from the gcj native libraries
+ * Some of this coded adopted from the gcj native libraries
  */
 
 #include <config.h>
@@ -90,10 +90,15 @@
 // FIXME: This can't be right.  Need converter macros
 #define CONVERT_JINT_TO_INT(x) ((int)(x & 0xFFFF))
 
-/* These are initialized in nativeInit() */
-static jint SET;
-static jint CUR;
-static jint END;
+/* These values must be kept in sync with FileDescriptor.java.  */
+#define SET 0
+#define CUR 1
+#define END 2
+#define READ 1
+#define WRITE 2
+#define APPEND 4
+#define EXCL 8
+
 
 /*************************************************************************/
 
@@ -104,14 +109,8 @@
 JNIEXPORT void JNICALL
 Java_java_io_FileDescriptor_nativeInit(JNIEnv *env, jclass clazz)
 {
-  /* FIXME: Should set these to same values as in the FileDescriptor java
-   * code */
-  SET = 0;
-  CUR = 1;
-  END = 2;
-
-  /* FIXME: If stdin, stdout, and stderr not fd 0, 1, 2, then set
-   * appropriate values to the static native fields in, out, err. */
+  /* FIXME: set appropriate values to the static native fields in,
+   * out, err. */
 }
 
 /*************************************************************************/
@@ -120,49 +119,55 @@
  */
 JNIEXPORT jlong JNICALL
 Java_java_io_FileDescriptor_nativeOpen(JNIEnv *env, jobject obj, jstring name, 
-                                       jstring mode)
+                                       jint jflags)
 {
   int rc;
-  char *cname, *cmode;
+  char *cname;
+  int flags;
+  int mode = 0666;             /* FIXME: use symbolic value */
 
   cname = JCL_jstring_to_cstring(env, name);
-  cmode = JCL_jstring_to_cstring(env, mode);
-  if (!cname || !cmode)
+  if (!cname)
     return(-1); /* Exception will already have been thrown */
 
-  // FIXME: Do we have the right permission mode?
-  if (!strcmp(cmode,"r"))
-    rc = open(cname, O_RDONLY);
-  else if (!strcmp(cmode,"w"))
-    rc = open(cname, O_WRONLY | O_CREAT,
-              S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
-  else if (!strcmp(cmode,"a"))
-    rc = open(cname, O_WRONLY | O_CREAT | O_APPEND,
-              S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
-  else if (!strcmp(cmode, "rw"))
-    rc = open(cname, O_RDWR | O_CREAT, 
-              S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
-  else if (!strcmp(cmode, "rwa"))
-    rc = open(cname, O_RDWR | O_CREAT | O_APPEND,
-              S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
-  else if (!strcmp(cmode, "rws") || !strcmp(cmode,"rwd"))
-    rc = open(cname, O_RDWR | O_CREAT | O_SYNC,
-              S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);
+  flags = 0;
+#ifdef O_BINARY
+  flags |= O_BINARY;
+#endif
+
+  /* We don't have to do any checking of the flags, as it is all done
+     in FileDescriptor.  We can assume everything is ok.  */
+  if ((jflags & READ) && (jflags & WRITE))
+    flags |= O_RDWR | O_CREAT;
+  else if ((jflags & READ))
+    flags |= O_RDONLY;
   else
-    rc = -1; /* Invalid mode */
+    {
+      flags |= O_WRONLY | O_CREAT;
+      if ((jflags & APPEND))
+       flags |= O_APPEND;
+      else
+       flags |= O_TRUNC;
 
-  (*env)->ReleaseStringUTFChars(env, name, cname);
-  (*env)->ReleaseStringUTFChars(env, mode, cmode);
+      if ((jflags & EXCL))
+       {
+         /* In this case we are making a temp file.  */
+         flags |= O_EXCL;
+         mode = 0600;
+       }
+    }
 
-  if (rc != -1)
-    return(rc);
+  rc = open (cname, flags, mode);
 
-  if (errno == ENOENT)
-    JCL_ThrowException(env,"java/io/FileNotFoundException", strerror(errno));
-  else
-    JCL_ThrowException(env,"java/io/IOException", strerror(errno)); 
+  (*env)->ReleaseStringUTFChars(env, name, cname);
 
-  return(rc);
+  if (rc == -1)
+    {
+      /* We can only throw FileNotFoundException.  */
+      JCL_ThrowException(env,"java/io/FileNotFoundException", strerror(errno));
+    }
+
+  return rc;
 }
 
 /*************************************************************************/
@@ -629,7 +634,6 @@
 /*************************************************************************/
 /*
  * Test file descriptor for validity
- * Exception on error
  */
 JNIEXPORT jboolean JNICALL
 Java_java_io_FileDescriptor_nativeValid(JNIEnv *env, jobject obj, jlong fd)