From 786cbcacd2efbd94476eb05a4d5b77211f20d434 Mon Sep 17 00:00:00 2001 From: Kenny Root Date: Thu, 16 Aug 2012 11:10:58 -0700 Subject: [PATCH 1/2] Use Libcore.os.stat instead of FileUtils PackageManagerService just needed to know the owner for this file, so just use stat instead so we can remove the old JNI code. This is the last user of FileUtils#getPermissions so just remove the FileUtils method as well. Change-Id: I953057cd6b9de4410f33b6f22e4bddff02fe2988 --- core/java/android/os/FileUtils.java | 9 ----- core/jni/android_os_FileUtils.cpp | 34 ------------------- .../server/pm/PackageManagerService.java | 23 +++++++------ 3 files changed, 13 insertions(+), 53 deletions(-) diff --git a/core/java/android/os/FileUtils.java b/core/java/android/os/FileUtils.java index 6c1445df74a0d..213e3ae79d406 100644 --- a/core/java/android/os/FileUtils.java +++ b/core/java/android/os/FileUtils.java @@ -28,9 +28,6 @@ import java.util.regex.Pattern; import java.util.zip.CRC32; import java.util.zip.CheckedInputStream; -import libcore.io.Os; -import libcore.io.StructStat; - /** * Tools for managing files. Not for public consumption. * @hide @@ -96,12 +93,6 @@ public class FileUtils { public static native int setPermissions(String file, int mode, int uid, int gid); - /** - * @deprecated use {@link Os#stat(String)} instead. - */ - @Deprecated - public static native int getPermissions(String file, int[] outPermissions); - public static native int setUMask(int mask); /** returns the FAT file system volume ID for the volume mounted diff --git a/core/jni/android_os_FileUtils.cpp b/core/jni/android_os_FileUtils.cpp index 8d65cbcf74a79..b1fce931a675b 100644 --- a/core/jni/android_os_FileUtils.cpp +++ b/core/jni/android_os_FileUtils.cpp @@ -68,39 +68,6 @@ jint android_os_FileUtils_setPermissions(JNIEnv* env, jobject clazz, return chmod(file8.string(), mode) == 0 ? 0 : errno; } -jint android_os_FileUtils_getPermissions(JNIEnv* env, jobject clazz, - jstring file, jintArray outArray) -{ - const jchar* str = env->GetStringCritical(file, 0); - String8 file8; - if (str) { - file8 = String8(str, env->GetStringLength(file)); - env->ReleaseStringCritical(file, str); - } - if (file8.size() <= 0) { - return ENOENT; - } - struct stat st; - if (stat(file8.string(), &st) != 0) { - return errno; - } - jint* array = (jint*)env->GetPrimitiveArrayCritical(outArray, 0); - if (array) { - int len = env->GetArrayLength(outArray); - if (len >= 1) { - array[0] = st.st_mode; - } - if (len >= 2) { - array[1] = st.st_uid; - } - if (len >= 3) { - array[2] = st.st_gid; - } - } - env->ReleasePrimitiveArrayCritical(outArray, array, 0); - return 0; -} - jint android_os_FileUtils_setUMask(JNIEnv* env, jobject clazz, jint mask) { return umask(mask); @@ -158,7 +125,6 @@ jboolean android_os_FileUtils_getFileStatus(JNIEnv* env, jobject clazz, jstring static const JNINativeMethod methods[] = { {"setPermissions", "(Ljava/lang/String;III)I", (void*)android_os_FileUtils_setPermissions}, - {"getPermissions", "(Ljava/lang/String;[I)I", (void*)android_os_FileUtils_getPermissions}, {"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}, diff --git a/services/java/com/android/server/pm/PackageManagerService.java b/services/java/com/android/server/pm/PackageManagerService.java index 8c5a0908c9b59..7390cbe2030fe 100644 --- a/services/java/com/android/server/pm/PackageManagerService.java +++ b/services/java/com/android/server/pm/PackageManagerService.java @@ -144,6 +144,7 @@ import java.util.Set; import libcore.io.ErrnoException; import libcore.io.IoUtils; import libcore.io.Libcore; +import libcore.io.StructStat; /** * Keep track of all those .apks everywhere. @@ -294,8 +295,6 @@ public class PackageManagerService extends IPackageManager.Stub { File mScanningPath; int mLastScanError; - final int[] mOutPermissions = new int[3]; - // ---------------------------------------------------------------- // Keys are String (package name), values are Package. This also serves @@ -3762,14 +3761,18 @@ public class PackageManagerService extends IPackageManager.Stub { boolean uidError = false; if (dataPath.exists()) { - // XXX should really do this check for each user. - mOutPermissions[1] = 0; - FileUtils.getPermissions(dataPath.getPath(), mOutPermissions); + int currentUid = 0; + try { + StructStat stat = Libcore.os.stat(dataPath.getPath()); + currentUid = stat.st_uid; + } catch (ErrnoException e) { + Slog.e(TAG, "Couldn't stat path " + dataPath.getPath(), e); + } // If we have mismatched owners for the data path, we have a problem. - if (mOutPermissions[1] != pkg.applicationInfo.uid) { + if (currentUid != pkg.applicationInfo.uid) { boolean recovered = false; - if (mOutPermissions[1] == 0) { + if (currentUid == 0) { // The directory somehow became owned by root. Wow. // This is probably because the system was stopped while // installd was in the middle of messing with its libs @@ -3798,7 +3801,7 @@ public class PackageManagerService extends IPackageManager.Stub { ? "System package " : "Third party package "; String msg = prefix + pkg.packageName + " has changed from uid: " - + mOutPermissions[1] + " to " + + currentUid + " to " + pkg.applicationInfo.uid + "; old data erased"; reportSettingsProblem(Log.WARN, msg); recovered = true; @@ -3830,11 +3833,11 @@ public class PackageManagerService extends IPackageManager.Stub { if (!recovered) { pkg.applicationInfo.dataDir = "/mismatched_uid/settings_" + pkg.applicationInfo.uid + "/fs_" - + mOutPermissions[1]; + + currentUid; pkg.applicationInfo.nativeLibraryDir = pkg.applicationInfo.dataDir; String msg = "Package " + pkg.packageName + " has mismatched uid: " - + mOutPermissions[1] + " on disk, " + + currentUid + " on disk, " + pkg.applicationInfo.uid + " in settings"; // writer synchronized (mPackages) { From 98e15e78934a00cf46f2be55472b7fd7a39ac0de Mon Sep 17 00:00:00 2001 From: Kenny Root Date: Thu, 16 Aug 2012 11:38:04 -0700 Subject: [PATCH 2/2] 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