From a233bbfcc80149b472c978ded50f9420936226fb Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Thu, 8 Jul 2010 11:02:37 -0700 Subject: [PATCH] Clean up OSFileSystem. I accidentally opened this file instead of OSNetworkSystem, but took the opportunity to move null pointer checking down into the JNI, where it doubles as OOM checking. I've also added missing null checks to the JNI. (Strictly, ScopedPrimitiveArray doesn't yet do the right thing in all cases, it's ScopedUtfChars that does. An improved ScopedPrimitiveArray will be along shortly...) Also some cosmetic tidying. Change-Id: Ie4d3a217332dae67a12db370246c7d87fe7edae4 --- .../apache/harmony/luni/platform/OSFileSystem.java | 110 +++++---------------- ...g_apache_harmony_luni_platform_OSFileSystem.cpp | 107 +++++++++++--------- 2 files changed, 81 insertions(+), 136 deletions(-) diff --git a/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java b/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java index 691f0649..c1ff96e7 100644 --- a/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java +++ b/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java @@ -24,12 +24,7 @@ package org.apache.harmony.luni.platform; import java.io.FileDescriptor; import java.io.FileNotFoundException; import java.io.IOException; -import java.io.UnsupportedEncodingException; -/** - * This is the portable implementation of the file system interface. - * - */ class OSFileSystem implements IFileSystem { private static final OSFileSystem singleton = new OSFileSystem(); @@ -39,7 +34,6 @@ class OSFileSystem implements IFileSystem { } private OSFileSystem() { - super(); } private final void validateLockArgs(int type, long start, long length) { @@ -50,19 +44,16 @@ class OSFileSystem implements IFileSystem { // Start position if (start < 0) { - throw new IllegalArgumentException( - "Lock start position must be non-negative"); + throw new IllegalArgumentException("start < 0"); } // Length of lock stretch if (length < 0) { - throw new IllegalArgumentException( - "Lock length must be non-negative"); + throw new IllegalArgumentException("length < 0"); } } - private native int lockImpl(int fileDescriptor, long start, long length, - int type, boolean wait); + private native int lockImpl(int fd, long start, long length, int type, boolean wait); /** * Returns the granularity for virtual memory allocation. @@ -73,25 +64,23 @@ class OSFileSystem implements IFileSystem { public native long length(int fd); - public boolean lock(int fileDescriptor, long start, long length, int type, - boolean waitFlag) throws IOException { + public boolean lock(int fd, long start, long length, int type, boolean waitFlag) + throws IOException { // Validate arguments validateLockArgs(type, start, length); - int result = lockImpl(fileDescriptor, start, length, type, waitFlag); + int result = lockImpl(fd, start, length, type, waitFlag); return result != -1; } - // BEGIN android-changed - private native void unlockImpl(int fileDescriptor, long start, long length) throws IOException; + private native void unlockImpl(int fd, long start, long length) throws IOException; - public void unlock(int fileDescriptor, long start, long length) - throws IOException { + public void unlock(int fd, long start, long length) throws IOException { // Validate arguments validateLockArgs(IFileSystem.SHARED_LOCK_TYPE, start, length); - unlockImpl(fileDescriptor, start, length); + unlockImpl(fd, start, length); } - public native void fflush(int fileDescriptor, boolean metadata) throws IOException; + public native void fflush(int fd, boolean metadata) throws IOException; /* * File position seeking. @@ -101,89 +90,34 @@ class OSFileSystem implements IFileSystem { /* * Direct read/write APIs work on addresses. */ - public native long readDirect(int fileDescriptor, int address, int offset, int length); + public native long readDirect(int fd, int address, int offset, int length); - public native long writeDirect(int fileDescriptor, int address, int offset, int length) - throws IOException; + public native long writeDirect(int fd, int address, int offset, int length); /* * Indirect read/writes work on byte[]'s */ - private native long readImpl(int fileDescriptor, byte[] bytes, int offset, - int length) throws IOException; + public native long read(int fd, byte[] bytes, int offset, int length) throws IOException; - public long read(int fileDescriptor, byte[] bytes, int offset, int length) - throws IOException { - if (bytes == null) { - throw new NullPointerException(); - } - return readImpl(fileDescriptor, bytes, offset, length); - } - - private native long writeImpl(int fileDescriptor, byte[] bytes, - int offset, int length) throws IOException; - - public long write(int fileDescriptor, byte[] bytes, int offset, int length) - throws IOException { - if (bytes == null) { - throw new NullPointerException(); - } - return writeImpl(fileDescriptor, bytes, offset, length); - } - // END android-changed + public native long write(int fd, byte[] bytes, int offset, int length) throws IOException; /* * Scatter/gather calls. */ - public native long readv(int fileDescriptor, int[] addresses, - int[] offsets, int[] lengths, int size) throws IOException; - - public native long writev(int fileDescriptor, int[] addresses, int[] offsets, - int[] lengths, int size) throws IOException; + public native long readv(int fd, int[] addresses, int[] offsets, int[] lengths, int size) + throws IOException; - // BEGIN android-changed - public native void close(int fileDescriptor) throws IOException; + public native long writev(int fd, int[] addresses, int[] offsets, int[] lengths, int size) + throws IOException; - public native void truncate(int fileDescriptor, long size) throws IOException; - // END android-changed + public native void close(int fd) throws IOException; - public int open(byte[] utfPathBytes, int mode) throws FileNotFoundException { - if (utfPathBytes == null) { - throw new NullPointerException(); - } - return openImpl(utfPathBytes, mode); - } + public native void truncate(int fd, long size) throws IOException; - private native int openImpl(byte[] fileName, int mode); + public native int open(byte[] utfPathBytes, int mode) throws FileNotFoundException; - // BEGIN android-changed public native long transfer(int fd, FileDescriptor sd, long offset, long count) throws IOException; - // END android-changed - - // BEGIN android-deleted - // public long ttyAvailable() throws IOException { - // long nChar = ttyAvailableImpl(); - // if (nChar < 0) { - // throw new IOException(); - // } - // return nChar; - // } - // - // private native long ttyAvailableImpl(); - // END android-deleted - - // BEGIN android-deleted - // public long ttyRead(byte[] bytes, int offset, int length) throws IOException { - // if (bytes == null) { - // throw new NullPointerException(); - // } - // return ttyReadImpl(bytes, offset, length); - // } - // private native long ttyReadImpl(byte[] bytes, int offset, int length) throws IOException; - // END android-deleted - - // BEGIN android-added + public native int ioctlAvailable(FileDescriptor fileDescriptor) throws IOException; - // END android-added } diff --git a/luni/src/main/native/org_apache_harmony_luni_platform_OSFileSystem.cpp b/luni/src/main/native/org_apache_harmony_luni_platform_OSFileSystem.cpp index 59c08cec..12f27016 100644 --- a/luni/src/main/native/org_apache_harmony_luni_platform_OSFileSystem.cpp +++ b/luni/src/main/native/org_apache_harmony_luni_platform_OSFileSystem.cpp @@ -60,8 +60,7 @@ */ #include #include -static inline ssize_t sendfile(int out_fd, int in_fd, off_t *offset, - size_t count) { +static inline ssize_t sendfile(int out_fd, int in_fd, off_t *offset, size_t count) { off_t len = count; int result = sendfile(in_fd, out_fd, *offset, &len, NULL, 0); if (result < 0) { @@ -146,7 +145,7 @@ static struct flock flockFromStartAndLength(jlong start, jlong length) { return lock; } -static jint harmony_io_lockImpl(JNIEnv* env, jobject, jint handle, +static jint OSFileSystem_lockImpl(JNIEnv* env, jobject, jint fd, jlong start, jlong length, jint typeFlag, jboolean waitFlag) { length = translateLockLength(length); @@ -163,12 +162,10 @@ static jint harmony_io_lockImpl(JNIEnv* env, jobject, jint handle, } int waitMode = (waitFlag) ? F_SETLKW : F_SETLK; - return TEMP_FAILURE_RETRY(fcntl(handle, waitMode, &lock)); + return TEMP_FAILURE_RETRY(fcntl(fd, waitMode, &lock)); } -static void harmony_io_unlockImpl(JNIEnv* env, jobject, jint handle, - jlong start, jlong length) { - +static void OSFileSystem_unlockImpl(JNIEnv* env, jobject, jint fd, jlong start, jlong length) { length = translateLockLength(length); if (offsetTooLarge(env, start) || offsetTooLarge(env, length)) { return; @@ -177,7 +174,7 @@ static void harmony_io_unlockImpl(JNIEnv* env, jobject, jint handle, struct flock lock(flockFromStartAndLength(start, length)); lock.l_type = F_UNLCK; - int rc = TEMP_FAILURE_RETRY(fcntl(handle, F_SETLKW, &lock)); + int rc = TEMP_FAILURE_RETRY(fcntl(fd, F_SETLKW, &lock)); if (rc == -1) { jniThrowIOException(env, errno); } @@ -187,7 +184,7 @@ static void harmony_io_unlockImpl(JNIEnv* env, jobject, jint handle, * Returns the granularity of the starting address for virtual memory allocation. * (It's the same as the page size.) */ -static jint harmony_io_getAllocGranularity(JNIEnv*, jobject) { +static jint OSFileSystem_getAllocGranularity(JNIEnv*, jobject) { static int allocGranularity = getpagesize(); return allocGranularity; } @@ -195,24 +192,33 @@ static jint harmony_io_getAllocGranularity(JNIEnv*, jobject) { // Translate three Java int[]s to a native iovec[] for readv and writev. static iovec* initIoVec(JNIEnv* env, jintArray jBuffers, jintArray jOffsets, jintArray jLengths, jint size) { - iovec* vectors = new iovec[size]; - if (vectors == NULL) { + UniquePtr vectors(new iovec[size]); + if (vectors.get() == NULL) { jniThrowException(env, "java/lang/OutOfMemoryError", "native heap"); return NULL; } ScopedIntArrayRO buffers(env, jBuffers); + if (buffers.get() == NULL) { + return NULL; + } ScopedIntArrayRO offsets(env, jOffsets); + if (offsets.get() == NULL) { + return NULL; + } ScopedIntArrayRO lengths(env, jLengths); + if (lengths.get() == NULL) { + return NULL; + } for (int i = 0; i < size; ++i) { vectors[i].iov_base = reinterpret_cast(buffers[i] + offsets[i]); vectors[i].iov_len = lengths[i]; } - return vectors; + return vectors.release(); } -static jlong harmony_io_readv(JNIEnv* env, jobject, jint fd, +static jlong OSFileSystem_readv(JNIEnv* env, jobject, jint fd, jintArray jBuffers, jintArray jOffsets, jintArray jLengths, jint size) { - UniquePtr vectors(initIoVec(env, jBuffers, jOffsets, jLengths, size)); + UniquePtr vectors(initIoVec(env, jBuffers, jOffsets, jLengths, size)); if (vectors.get() == NULL) { return -1; } @@ -226,9 +232,9 @@ static jlong harmony_io_readv(JNIEnv* env, jobject, jint fd, return result; } -static jlong harmony_io_writev(JNIEnv* env, jobject, jint fd, +static jlong OSFileSystem_writev(JNIEnv* env, jobject, jint fd, jintArray jBuffers, jintArray jOffsets, jintArray jLengths, jint size) { - UniquePtr vectors(initIoVec(env, jBuffers, jOffsets, jLengths, size)); + UniquePtr vectors(initIoVec(env, jBuffers, jOffsets, jLengths, size)); if (vectors.get() == NULL) { return -1; } @@ -239,7 +245,7 @@ static jlong harmony_io_writev(JNIEnv* env, jobject, jint fd, return result; } -static jlong harmony_io_transfer(JNIEnv* env, jobject, jint fd, jobject sd, +static jlong OSFileSystem_transfer(JNIEnv* env, jobject, jint fd, jobject sd, jlong offset, jlong count) { int socket = jniGetFDFromFileDescriptor(env, sd); @@ -259,7 +265,7 @@ static jlong harmony_io_transfer(JNIEnv* env, jobject, jint fd, jobject sd, return rc; } -static jlong harmony_io_readDirect(JNIEnv* env, jobject, jint fd, +static jlong OSFileSystem_readDirect(JNIEnv* env, jobject, jint fd, jint buf, jint offset, jint nbytes) { if (nbytes == 0) { return 0; @@ -276,7 +282,7 @@ static jlong harmony_io_readDirect(JNIEnv* env, jobject, jint fd, return rc; } -static jlong harmony_io_writeDirect(JNIEnv* env, jobject, jint fd, +static jlong OSFileSystem_writeDirect(JNIEnv* env, jobject, jint fd, jint buf, jint offset, jint nbytes) { jbyte* src = reinterpret_cast(buf + offset); jlong rc = TEMP_FAILURE_RETRY(write(fd, src, nbytes)); @@ -286,14 +292,16 @@ static jlong harmony_io_writeDirect(JNIEnv* env, jobject, jint fd, return rc; } -static jlong harmony_io_readImpl(JNIEnv* env, jobject, jint fd, +static jlong OSFileSystem_read(JNIEnv* env, jobject, jint fd, jbyteArray byteArray, jint offset, jint nbytes) { - if (nbytes == 0) { return 0; } ScopedByteArrayRW bytes(env, byteArray); + if (bytes.get() == NULL) { + return 0; + } jlong rc = TEMP_FAILURE_RETRY(read(fd, bytes.get() + offset, nbytes)); if (rc == 0) { return -1; @@ -308,15 +316,17 @@ static jlong harmony_io_readImpl(JNIEnv* env, jobject, jint fd, return rc; } -static jlong harmony_io_writeImpl(JNIEnv* env, jobject, jint fd, +static jlong OSFileSystem_write(JNIEnv* env, jobject, jint fd, jbyteArray byteArray, jint offset, jint nbytes) { ScopedByteArrayRO bytes(env, byteArray); + if (bytes.get() == NULL) { + return 0; + } jlong result = TEMP_FAILURE_RETRY(write(fd, bytes.get() + offset, nbytes)); if (result == -1) { if (errno == EAGAIN) { - jniThrowException(env, "java/io/InterruptedIOException", - "Write timed out"); + jniThrowException(env, "java/io/InterruptedIOException", "Write timed out"); } else { jniThrowIOException(env, errno); } @@ -324,8 +334,7 @@ static jlong harmony_io_writeImpl(JNIEnv* env, jobject, jint fd, return result; } -static jlong harmony_io_seek(JNIEnv* env, jobject, jint fd, jlong offset, - jint javaWhence) { +static jlong OSFileSystem_seek(JNIEnv* env, jobject, jint fd, jlong offset, jint javaWhence) { /* Convert whence argument */ int nativeWhence = 0; switch (javaWhence) { @@ -356,7 +365,7 @@ static jlong harmony_io_seek(JNIEnv* env, jobject, jint fd, jlong offset, return result; } -static void harmony_io_fflush(JNIEnv* env, jobject, jint fd, jboolean metadataToo) { +static void OSFileSystem_fflush(JNIEnv* env, jobject, jint fd, jboolean metadataToo) { LOGW("fdatasync unimplemented on Android"); // http://b/2667481 int rc = fsync(fd); // int rc = metadataToo ? fsync(fd) : fdatasync(fd); @@ -365,7 +374,7 @@ static void harmony_io_fflush(JNIEnv* env, jobject, jint fd, jboolean metadataTo } } -static jint harmony_io_close(JNIEnv* env, jobject, jint fd) { +static jint OSFileSystem_close(JNIEnv* env, jobject, jint fd) { jint rc = TEMP_FAILURE_RETRY(close(fd)); if (rc == -1) { jniThrowIOException(env, errno); @@ -373,7 +382,7 @@ static jint harmony_io_close(JNIEnv* env, jobject, jint fd) { return rc; } -static jint harmony_io_truncate(JNIEnv* env, jobject, jint fd, jlong length) { +static jint OSFileSystem_truncate(JNIEnv* env, jobject, jint fd, jlong length) { if (offsetTooLarge(env, length)) { return -1; } @@ -385,8 +394,7 @@ static jint harmony_io_truncate(JNIEnv* env, jobject, jint fd, jlong length) { return rc; } -static jint harmony_io_openImpl(JNIEnv* env, jobject, jbyteArray pathByteArray, - jint jflags) { +static jint OSFileSystem_open(JNIEnv* env, jobject, jbyteArray pathByteArray, jint jflags) { int flags = 0; int mode = 0; @@ -419,6 +427,9 @@ static jint harmony_io_openImpl(JNIEnv* env, jobject, jbyteArray pathByteArray, flags = EsTranslateOpenFlags(flags); ScopedByteArrayRO path(env, pathByteArray); + if (path.get() == NULL) { + return -1; + } jint rc = TEMP_FAILURE_RETRY(open(reinterpret_cast(&path[0]), flags, mode)); if (rc == -1) { // Get the human-readable form of errno. @@ -437,7 +448,7 @@ static jint harmony_io_openImpl(JNIEnv* env, jobject, jbyteArray pathByteArray, return rc; } -static jint harmony_io_ioctlAvailable(JNIEnv*env, jobject, jobject fileDescriptor) { +static jint OSFileSystem_ioctlAvailable(JNIEnv*env, jobject, jobject fileDescriptor) { /* * On underlying platforms Android cares about (read "Linux"), * ioctl(fd, FIONREAD, &avail) is supposed to do the following: @@ -498,23 +509,23 @@ static jlong lengthImpl(JNIEnv* env, jobject, jint fd) { } static JNINativeMethod gMethods[] = { - { "close", "(I)V", (void*) harmony_io_close }, - { "fflush", "(IZ)V", (void*) harmony_io_fflush }, - { "getAllocGranularity","()I", (void*) harmony_io_getAllocGranularity }, - { "ioctlAvailable", "(Ljava/io/FileDescriptor;)I", (void*) harmony_io_ioctlAvailable }, + { "close", "(I)V", (void*) OSFileSystem_close }, + { "fflush", "(IZ)V", (void*) OSFileSystem_fflush }, + { "getAllocGranularity", "()I", (void*) OSFileSystem_getAllocGranularity }, + { "ioctlAvailable", "(Ljava/io/FileDescriptor;)I", (void*) OSFileSystem_ioctlAvailable }, { "length", "(I)J", (void*) lengthImpl }, - { "lockImpl", "(IJJIZ)I", (void*) harmony_io_lockImpl }, - { "openImpl", "([BI)I", (void*) harmony_io_openImpl }, - { "readDirect", "(IIII)J", (void*) harmony_io_readDirect }, - { "readImpl", "(I[BII)J", (void*) harmony_io_readImpl }, - { "readv", "(I[I[I[II)J",(void*) harmony_io_readv }, - { "seek", "(IJI)J", (void*) harmony_io_seek }, - { "transfer", "(ILjava/io/FileDescriptor;JJ)J", (void*) harmony_io_transfer }, - { "truncate", "(IJ)V", (void*) harmony_io_truncate }, - { "unlockImpl", "(IJJ)V", (void*) harmony_io_unlockImpl }, - { "writeDirect", "(IIII)J", (void*) harmony_io_writeDirect }, - { "writeImpl", "(I[BII)J", (void*) harmony_io_writeImpl }, - { "writev", "(I[I[I[II)J",(void*) harmony_io_writev }, + { "lockImpl", "(IJJIZ)I", (void*) OSFileSystem_lockImpl }, + { "open", "([BI)I", (void*) OSFileSystem_open }, + { "read", "(I[BII)J", (void*) OSFileSystem_read }, + { "readDirect", "(IIII)J", (void*) OSFileSystem_readDirect }, + { "readv", "(I[I[I[II)J", (void*) OSFileSystem_readv }, + { "seek", "(IJI)J", (void*) OSFileSystem_seek }, + { "transfer", "(ILjava/io/FileDescriptor;JJ)J", (void*) OSFileSystem_transfer }, + { "truncate", "(IJ)V", (void*) OSFileSystem_truncate }, + { "unlockImpl", "(IJJ)V", (void*) OSFileSystem_unlockImpl }, + { "write", "(I[BII)J", (void*) OSFileSystem_write }, + { "writeDirect", "(IIII)J", (void*) OSFileSystem_writeDirect }, + { "writev", "(I[I[I[II)J", (void*) OSFileSystem_writev }, }; int register_org_apache_harmony_luni_platform_OSFileSystem(JNIEnv* env) { return jniRegisterNativeMethods(env, "org/apache/harmony/luni/platform/OSFileSystem", gMethods, -- 2.11.0