From 98e15e78934a00cf46f2be55472b7fd7a39ac0de Mon Sep 17 00:00:00 2001 From: Kenny Root Date: Thu, 16 Aug 2012 11:38:04 -0700 Subject: [PATCH] Use Libcore's stat instead of FileUtils#getFileStatus Remove the last user of FileUtils#getFileStatus and move it to Libcore.os.stat instead. Then we can remove the JNI code that does the equivalent of a stat. Change-Id: Ieb566a2a8a17c2dd0150724b4eb3ac1cc41c823d --- .../android/app/SharedPreferencesImpl.java | 67 ++++++++++++------- core/java/android/os/FileUtils.java | 40 ----------- core/jni/android_os_FileUtils.cpp | 59 ---------------- .../src/android/os/FileUtilsTest.java | 64 +----------------- 4 files changed, 45 insertions(+), 185 deletions(-) diff --git a/core/java/android/app/SharedPreferencesImpl.java b/core/java/android/app/SharedPreferencesImpl.java index 615e8cea64c48..201d7b267f1a3 100644 --- a/core/java/android/app/SharedPreferencesImpl.java +++ b/core/java/android/app/SharedPreferencesImpl.java @@ -17,7 +17,6 @@ package android.app; import android.content.SharedPreferences; -import android.os.FileUtils.FileStatus; import android.os.FileUtils; import android.os.Looper; import android.util.Log; @@ -45,6 +44,11 @@ import java.util.WeakHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; +import libcore.io.ErrnoException; +import libcore.io.IoUtils; +import libcore.io.Libcore; +import libcore.io.StructStat; + final class SharedPreferencesImpl implements SharedPreferences { private static final String TAG = "SharedPreferencesImpl"; private static final boolean DEBUG = false; @@ -105,26 +109,32 @@ final class SharedPreferencesImpl implements SharedPreferences { } Map map = null; - FileStatus stat = new FileStatus(); - if (FileUtils.getFileStatus(mFile.getPath(), stat) && mFile.canRead()) { - try { - BufferedInputStream str = new BufferedInputStream( - new FileInputStream(mFile), 16*1024); - map = XmlUtils.readMapXml(str); - str.close(); - } catch (XmlPullParserException e) { - Log.w(TAG, "getSharedPreferences", e); - } catch (FileNotFoundException e) { - Log.w(TAG, "getSharedPreferences", e); - } catch (IOException e) { - Log.w(TAG, "getSharedPreferences", e); + StructStat stat = null; + try { + stat = Libcore.os.stat(mFile.getPath()); + if (mFile.canRead()) { + BufferedInputStream str = null; + try { + str = new BufferedInputStream( + new FileInputStream(mFile), 16*1024); + map = XmlUtils.readMapXml(str); + } catch (XmlPullParserException e) { + Log.w(TAG, "getSharedPreferences", e); + } catch (FileNotFoundException e) { + Log.w(TAG, "getSharedPreferences", e); + } catch (IOException e) { + Log.w(TAG, "getSharedPreferences", e); + } finally { + IoUtils.closeQuietly(str); + } } + } catch (ErrnoException e) { } mLoaded = true; if (map != null) { mMap = map; - mStatTimestamp = stat.mtime; - mStatSize = stat.size; + mStatTimestamp = stat.st_mtime; + mStatSize = stat.st_size; } else { mMap = new HashMap(); } @@ -155,12 +165,21 @@ final class SharedPreferencesImpl implements SharedPreferences { return false; } } - FileStatus stat = new FileStatus(); - if (!FileUtils.getFileStatus(mFile.getPath(), stat)) { + + final StructStat stat; + try { + /* + * Metadata operations don't usually count as a block guard + * violation, but we explicitly want this one. + */ + BlockGuard.getThreadPolicy().onReadFromDisk(); + stat = Libcore.os.stat(mFile.getPath()); + } catch (ErrnoException e) { return true; } + synchronized (this) { - return mStatTimestamp != stat.mtime || mStatSize != stat.size; + return mStatTimestamp != stat.st_mtime || mStatSize != stat.st_size; } } @@ -577,12 +596,14 @@ final class SharedPreferencesImpl implements SharedPreferences { FileUtils.sync(str); str.close(); ContextImpl.setFilePermissionsFromMode(mFile.getPath(), mMode, 0); - FileStatus stat = new FileStatus(); - if (FileUtils.getFileStatus(mFile.getPath(), stat)) { + try { + final StructStat stat = Libcore.os.stat(mFile.getPath()); synchronized (this) { - mStatTimestamp = stat.mtime; - mStatSize = stat.size; + mStatTimestamp = stat.st_mtime; + mStatSize = stat.st_size; } + } catch (ErrnoException e) { + // Do nothing } // Writing was successful, delete the backup file if there is one. mBackupFile.delete(); diff --git a/core/java/android/os/FileUtils.java b/core/java/android/os/FileUtils.java index 213e3ae79d406..7c103aaa1484d 100644 --- a/core/java/android/os/FileUtils.java +++ b/core/java/android/os/FileUtils.java @@ -47,46 +47,6 @@ public class FileUtils { public static final int S_IROTH = 00004; public static final int S_IWOTH = 00002; public static final int S_IXOTH = 00001; - - - /** - * File status information. This class maps directly to the POSIX stat structure. - * @deprecated use {@link StructStat} instead. - * @hide - */ - @Deprecated - public static final class FileStatus { - public int dev; - public int ino; - public int mode; - public int nlink; - public int uid; - public int gid; - public int rdev; - public long size; - public int blksize; - public long blocks; - public long atime; - public long mtime; - public long ctime; - } - - /** - * Get the status for the given path. This is equivalent to the POSIX stat(2) system call. - * @param path The path of the file to be stat'd. - * @param status Optional argument to fill in. It will only fill in the status if the file - * exists. - * @return true if the file exists and false if it does not exist. If you do not have - * permission to stat the file, then this method will return false. - * @deprecated use {@link Os#stat(String)} instead. - */ - @Deprecated - public static boolean getFileStatus(String path, FileStatus status) { - StrictMode.noteDiskRead(); - return getFileStatusNative(path, status); - } - - private static native boolean getFileStatusNative(String path, FileStatus status); /** Regular expression for safe filenames: no spaces or metacharacters */ private static final Pattern SAFE_FILENAME_PATTERN = Pattern.compile("[\\w%+,./=_-]+"); diff --git a/core/jni/android_os_FileUtils.cpp b/core/jni/android_os_FileUtils.cpp index b1fce931a675b..82bfc3646b463 100644 --- a/core/jni/android_os_FileUtils.cpp +++ b/core/jni/android_os_FileUtils.cpp @@ -33,19 +33,6 @@ namespace android { -static jfieldID gFileStatusDevFieldID; -static jfieldID gFileStatusInoFieldID; -static jfieldID gFileStatusModeFieldID; -static jfieldID gFileStatusNlinkFieldID; -static jfieldID gFileStatusUidFieldID; -static jfieldID gFileStatusGidFieldID; -static jfieldID gFileStatusSizeFieldID; -static jfieldID gFileStatusBlksizeFieldID; -static jfieldID gFileStatusBlocksFieldID; -static jfieldID gFileStatusAtimeFieldID; -static jfieldID gFileStatusMtimeFieldID; -static jfieldID gFileStatusCtimeFieldID; - jint android_os_FileUtils_setPermissions(JNIEnv* env, jobject clazz, jstring file, jint mode, jint uid, jint gid) @@ -94,62 +81,16 @@ jint android_os_FileUtils_getFatVolumeId(JNIEnv* env, jobject clazz, jstring pat return result; } -jboolean android_os_FileUtils_getFileStatus(JNIEnv* env, jobject clazz, jstring path, jobject fileStatus) { - const char* pathStr = env->GetStringUTFChars(path, NULL); - jboolean ret = false; - - struct stat s; - int res = stat(pathStr, &s); - if (res == 0) { - ret = true; - if (fileStatus != NULL) { - env->SetIntField(fileStatus, gFileStatusDevFieldID, s.st_dev); - env->SetIntField(fileStatus, gFileStatusInoFieldID, s.st_ino); - env->SetIntField(fileStatus, gFileStatusModeFieldID, s.st_mode); - env->SetIntField(fileStatus, gFileStatusNlinkFieldID, s.st_nlink); - env->SetIntField(fileStatus, gFileStatusUidFieldID, s.st_uid); - env->SetIntField(fileStatus, gFileStatusGidFieldID, s.st_gid); - env->SetLongField(fileStatus, gFileStatusSizeFieldID, s.st_size); - env->SetIntField(fileStatus, gFileStatusBlksizeFieldID, s.st_blksize); - env->SetLongField(fileStatus, gFileStatusBlocksFieldID, s.st_blocks); - env->SetLongField(fileStatus, gFileStatusAtimeFieldID, s.st_atime); - env->SetLongField(fileStatus, gFileStatusMtimeFieldID, s.st_mtime); - env->SetLongField(fileStatus, gFileStatusCtimeFieldID, s.st_ctime); - } - } - - env->ReleaseStringUTFChars(path, pathStr); - - return ret; -} - static const JNINativeMethod methods[] = { {"setPermissions", "(Ljava/lang/String;III)I", (void*)android_os_FileUtils_setPermissions}, {"setUMask", "(I)I", (void*)android_os_FileUtils_setUMask}, {"getFatVolumeId", "(Ljava/lang/String;)I", (void*)android_os_FileUtils_getFatVolumeId}, - {"getFileStatusNative", "(Ljava/lang/String;Landroid/os/FileUtils$FileStatus;)Z", (void*)android_os_FileUtils_getFileStatus}, }; static const char* const kFileUtilsPathName = "android/os/FileUtils"; int register_android_os_FileUtils(JNIEnv* env) { - jclass fileStatusClass = env->FindClass("android/os/FileUtils$FileStatus"); - LOG_FATAL_IF(fileStatusClass == NULL, "Unable to find class android.os.FileUtils$FileStatus"); - - gFileStatusDevFieldID = env->GetFieldID(fileStatusClass, "dev", "I"); - gFileStatusInoFieldID = env->GetFieldID(fileStatusClass, "ino", "I"); - gFileStatusModeFieldID = env->GetFieldID(fileStatusClass, "mode", "I"); - gFileStatusNlinkFieldID = env->GetFieldID(fileStatusClass, "nlink", "I"); - gFileStatusUidFieldID = env->GetFieldID(fileStatusClass, "uid", "I"); - gFileStatusGidFieldID = env->GetFieldID(fileStatusClass, "gid", "I"); - gFileStatusSizeFieldID = env->GetFieldID(fileStatusClass, "size", "J"); - gFileStatusBlksizeFieldID = env->GetFieldID(fileStatusClass, "blksize", "I"); - gFileStatusBlocksFieldID = env->GetFieldID(fileStatusClass, "blocks", "J"); - gFileStatusAtimeFieldID = env->GetFieldID(fileStatusClass, "atime", "J"); - gFileStatusMtimeFieldID = env->GetFieldID(fileStatusClass, "mtime", "J"); - gFileStatusCtimeFieldID = env->GetFieldID(fileStatusClass, "ctime", "J"); - return AndroidRuntime::registerNativeMethods( env, kFileUtilsPathName, methods, NELEM(methods)); diff --git a/core/tests/coretests/src/android/os/FileUtilsTest.java b/core/tests/coretests/src/android/os/FileUtilsTest.java index f12cbe1e7983d..4d0b892584127 100644 --- a/core/tests/coretests/src/android/os/FileUtilsTest.java +++ b/core/tests/coretests/src/android/os/FileUtilsTest.java @@ -17,21 +17,13 @@ package android.os; import android.content.Context; -import android.os.FileUtils; -import android.os.FileUtils.FileStatus; import android.test.AndroidTestCase; -import android.test.suitebuilder.annotation.LargeTest; import android.test.suitebuilder.annotation.MediumTest; -import android.test.suitebuilder.annotation.SmallTest; import java.io.ByteArrayInputStream; import java.io.File; -import java.io.FileWriter; -import java.io.FileNotFoundException; import java.io.FileOutputStream; -import java.io.IOException; - -import junit.framework.Assert; +import java.io.FileWriter; public class FileUtilsTest extends AndroidTestCase { private static final String TEST_DATA = @@ -60,60 +52,6 @@ public class FileUtilsTest extends AndroidTestCase { if (mCopyFile.exists()) mCopyFile.delete(); } - @LargeTest - public void testGetFileStatus() { - final byte[] MAGIC = { 0xB, 0xE, 0x0, 0x5 }; - - try { - // truncate test file and write MAGIC (4 bytes) to it. - FileOutputStream os = new FileOutputStream(mTestFile, false); - os.write(MAGIC, 0, 4); - os.flush(); - os.close(); - } catch (FileNotFoundException e) { - Assert.fail("File was removed durning test" + e); - } catch (IOException e) { - Assert.fail("Unexpected IOException: " + e); - } - - Assert.assertTrue(mTestFile.exists()); - Assert.assertTrue(FileUtils.getFileStatus(mTestFile.getPath(), null)); - - FileStatus status1 = new FileStatus(); - FileUtils.getFileStatus(mTestFile.getPath(), status1); - - Assert.assertEquals(4, status1.size); - - // Sleep for at least one second so that the modification time will be different. - try { - Thread.sleep(1000); - } catch (InterruptedException e) { - } - - try { - // append so we don't change the creation time. - FileOutputStream os = new FileOutputStream(mTestFile, true); - os.write(MAGIC, 0, 4); - os.flush(); - os.close(); - } catch (FileNotFoundException e) { - Assert.fail("File was removed durning test" + e); - } catch (IOException e) { - Assert.fail("Unexpected IOException: " + e); - } - - FileStatus status2 = new FileStatus(); - FileUtils.getFileStatus(mTestFile.getPath(), status2); - - Assert.assertEquals(8, status2.size); - Assert.assertTrue(status2.mtime > status1.mtime); - - mTestFile.delete(); - - Assert.assertFalse(mTestFile.exists()); - Assert.assertFalse(FileUtils.getFileStatus(mTestFile.getPath(), null)); - } - // TODO: test setPermissions(), getPermissions() @MediumTest