Skip to content

Commit a950daf

Browse files
krutonandroid code review
kruton
authored and
android code review
committed
Merge changes Ieb566a2a,I953057cd
* changes: Use Libcore's stat instead of FileUtils#getFileStatus Use Libcore.os.stat instead of FileUtils
2 parents d2fb6e9 + 98e15e7 commit a950daf

5 files changed

Lines changed: 58 additions & 238 deletions

File tree

‎core/java/android/app/SharedPreferencesImpl.java‎

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package android.app;
1818

1919
import android.content.SharedPreferences;
20-
import android.os.FileUtils.FileStatus;
2120
import android.os.FileUtils;
2221
import android.os.Looper;
2322
import android.util.Log;
@@ -45,6 +44,11 @@
4544
import java.util.concurrent.CountDownLatch;
4645
import java.util.concurrent.ExecutorService;
4746

47+
import libcore.io.ErrnoException;
48+
import libcore.io.IoUtils;
49+
import libcore.io.Libcore;
50+
import libcore.io.StructStat;
51+
4852
final class SharedPreferencesImpl implements SharedPreferences {
4953
private static final String TAG = "SharedPreferencesImpl";
5054
private static final boolean DEBUG = false;
@@ -105,26 +109,32 @@ private void loadFromDiskLocked() {
105109
}
106110

107111
Map map = null;
108-
FileStatus stat = new FileStatus();
109-
if (FileUtils.getFileStatus(mFile.getPath(), stat) && mFile.canRead()) {
110-
try {
111-
BufferedInputStream str = new BufferedInputStream(
112-
new FileInputStream(mFile), 16*1024);
113-
map = XmlUtils.readMapXml(str);
114-
str.close();
115-
} catch (XmlPullParserException e) {
116-
Log.w(TAG, "getSharedPreferences", e);
117-
} catch (FileNotFoundException e) {
118-
Log.w(TAG, "getSharedPreferences", e);
119-
} catch (IOException e) {
120-
Log.w(TAG, "getSharedPreferences", e);
112+
StructStat stat = null;
113+
try {
114+
stat = Libcore.os.stat(mFile.getPath());
115+
if (mFile.canRead()) {
116+
BufferedInputStream str = null;
117+
try {
118+
str = new BufferedInputStream(
119+
new FileInputStream(mFile), 16*1024);
120+
map = XmlUtils.readMapXml(str);
121+
} catch (XmlPullParserException e) {
122+
Log.w(TAG, "getSharedPreferences", e);
123+
} catch (FileNotFoundException e) {
124+
Log.w(TAG, "getSharedPreferences", e);
125+
} catch (IOException e) {
126+
Log.w(TAG, "getSharedPreferences", e);
127+
} finally {
128+
IoUtils.closeQuietly(str);
129+
}
121130
}
131+
} catch (ErrnoException e) {
122132
}
123133
mLoaded = true;
124134
if (map != null) {
125135
mMap = map;
126-
mStatTimestamp = stat.mtime;
127-
mStatSize = stat.size;
136+
mStatTimestamp = stat.st_mtime;
137+
mStatSize = stat.st_size;
128138
} else {
129139
mMap = new HashMap<String, Object>();
130140
}
@@ -155,12 +165,21 @@ private boolean hasFileChangedUnexpectedly() {
155165
return false;
156166
}
157167
}
158-
FileStatus stat = new FileStatus();
159-
if (!FileUtils.getFileStatus(mFile.getPath(), stat)) {
168+
169+
final StructStat stat;
170+
try {
171+
/*
172+
* Metadata operations don't usually count as a block guard
173+
* violation, but we explicitly want this one.
174+
*/
175+
BlockGuard.getThreadPolicy().onReadFromDisk();
176+
stat = Libcore.os.stat(mFile.getPath());
177+
} catch (ErrnoException e) {
160178
return true;
161179
}
180+
162181
synchronized (this) {
163-
return mStatTimestamp != stat.mtime || mStatSize != stat.size;
182+
return mStatTimestamp != stat.st_mtime || mStatSize != stat.st_size;
164183
}
165184
}
166185

@@ -577,12 +596,14 @@ private void writeToFile(MemoryCommitResult mcr) {
577596
FileUtils.sync(str);
578597
str.close();
579598
ContextImpl.setFilePermissionsFromMode(mFile.getPath(), mMode, 0);
580-
FileStatus stat = new FileStatus();
581-
if (FileUtils.getFileStatus(mFile.getPath(), stat)) {
599+
try {
600+
final StructStat stat = Libcore.os.stat(mFile.getPath());
582601
synchronized (this) {
583-
mStatTimestamp = stat.mtime;
584-
mStatSize = stat.size;
602+
mStatTimestamp = stat.st_mtime;
603+
mStatSize = stat.st_size;
585604
}
605+
} catch (ErrnoException e) {
606+
// Do nothing
586607
}
587608
// Writing was successful, delete the backup file if there is one.
588609
mBackupFile.delete();

‎core/java/android/os/FileUtils.java‎

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@
2828
import java.util.zip.CRC32;
2929
import java.util.zip.CheckedInputStream;
3030

31-
import libcore.io.Os;
32-
import libcore.io.StructStat;
33-
3431
/**
3532
* Tools for managing files. Not for public consumption.
3633
* @hide
@@ -50,58 +47,12 @@ public class FileUtils {
5047
public static final int S_IROTH = 00004;
5148
public static final int S_IWOTH = 00002;
5249
public static final int S_IXOTH = 00001;
53-
54-
55-
/**
56-
* File status information. This class maps directly to the POSIX stat structure.
57-
* @deprecated use {@link StructStat} instead.
58-
* @hide
59-
*/
60-
@Deprecated
61-
public static final class FileStatus {
62-
public int dev;
63-
public int ino;
64-
public int mode;
65-
public int nlink;
66-
public int uid;
67-
public int gid;
68-
public int rdev;
69-
public long size;
70-
public int blksize;
71-
public long blocks;
72-
public long atime;
73-
public long mtime;
74-
public long ctime;
75-
}
76-
77-
/**
78-
* Get the status for the given path. This is equivalent to the POSIX stat(2) system call.
79-
* @param path The path of the file to be stat'd.
80-
* @param status Optional argument to fill in. It will only fill in the status if the file
81-
* exists.
82-
* @return true if the file exists and false if it does not exist. If you do not have
83-
* permission to stat the file, then this method will return false.
84-
* @deprecated use {@link Os#stat(String)} instead.
85-
*/
86-
@Deprecated
87-
public static boolean getFileStatus(String path, FileStatus status) {
88-
StrictMode.noteDiskRead();
89-
return getFileStatusNative(path, status);
90-
}
91-
92-
private static native boolean getFileStatusNative(String path, FileStatus status);
9350

9451
/** Regular expression for safe filenames: no spaces or metacharacters */
9552
private static final Pattern SAFE_FILENAME_PATTERN = Pattern.compile("[\\w%+,./=_-]+");
9653

9754
public static native int setPermissions(String file, int mode, int uid, int gid);
9855

99-
/**
100-
* @deprecated use {@link Os#stat(String)} instead.
101-
*/
102-
@Deprecated
103-
public static native int getPermissions(String file, int[] outPermissions);
104-
10556
public static native int setUMask(int mask);
10657

10758
/** returns the FAT file system volume ID for the volume mounted

‎core/jni/android_os_FileUtils.cpp‎

Lines changed: 0 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,6 @@
3333

3434
namespace android {
3535

36-
static jfieldID gFileStatusDevFieldID;
37-
static jfieldID gFileStatusInoFieldID;
38-
static jfieldID gFileStatusModeFieldID;
39-
static jfieldID gFileStatusNlinkFieldID;
40-
static jfieldID gFileStatusUidFieldID;
41-
static jfieldID gFileStatusGidFieldID;
42-
static jfieldID gFileStatusSizeFieldID;
43-
static jfieldID gFileStatusBlksizeFieldID;
44-
static jfieldID gFileStatusBlocksFieldID;
45-
static jfieldID gFileStatusAtimeFieldID;
46-
static jfieldID gFileStatusMtimeFieldID;
47-
static jfieldID gFileStatusCtimeFieldID;
48-
4936
jint android_os_FileUtils_setPermissions(JNIEnv* env, jobject clazz,
5037
jstring file, jint mode,
5138
jint uid, jint gid)
@@ -68,39 +55,6 @@ jint android_os_FileUtils_setPermissions(JNIEnv* env, jobject clazz,
6855
return chmod(file8.string(), mode) == 0 ? 0 : errno;
6956
}
7057

71-
jint android_os_FileUtils_getPermissions(JNIEnv* env, jobject clazz,
72-
jstring file, jintArray outArray)
73-
{
74-
const jchar* str = env->GetStringCritical(file, 0);
75-
String8 file8;
76-
if (str) {
77-
file8 = String8(str, env->GetStringLength(file));
78-
env->ReleaseStringCritical(file, str);
79-
}
80-
if (file8.size() <= 0) {
81-
return ENOENT;
82-
}
83-
struct stat st;
84-
if (stat(file8.string(), &st) != 0) {
85-
return errno;
86-
}
87-
jint* array = (jint*)env->GetPrimitiveArrayCritical(outArray, 0);
88-
if (array) {
89-
int len = env->GetArrayLength(outArray);
90-
if (len >= 1) {
91-
array[0] = st.st_mode;
92-
}
93-
if (len >= 2) {
94-
array[1] = st.st_uid;
95-
}
96-
if (len >= 3) {
97-
array[2] = st.st_gid;
98-
}
99-
}
100-
env->ReleasePrimitiveArrayCritical(outArray, array, 0);
101-
return 0;
102-
}
103-
10458
jint android_os_FileUtils_setUMask(JNIEnv* env, jobject clazz, jint mask)
10559
{
10660
return umask(mask);
@@ -127,63 +81,16 @@ jint android_os_FileUtils_getFatVolumeId(JNIEnv* env, jobject clazz, jstring pat
12781
return result;
12882
}
12983

130-
jboolean android_os_FileUtils_getFileStatus(JNIEnv* env, jobject clazz, jstring path, jobject fileStatus) {
131-
const char* pathStr = env->GetStringUTFChars(path, NULL);
132-
jboolean ret = false;
133-
134-
struct stat s;
135-
int res = stat(pathStr, &s);
136-
if (res == 0) {
137-
ret = true;
138-
if (fileStatus != NULL) {
139-
env->SetIntField(fileStatus, gFileStatusDevFieldID, s.st_dev);
140-
env->SetIntField(fileStatus, gFileStatusInoFieldID, s.st_ino);
141-
env->SetIntField(fileStatus, gFileStatusModeFieldID, s.st_mode);
142-
env->SetIntField(fileStatus, gFileStatusNlinkFieldID, s.st_nlink);
143-
env->SetIntField(fileStatus, gFileStatusUidFieldID, s.st_uid);
144-
env->SetIntField(fileStatus, gFileStatusGidFieldID, s.st_gid);
145-
env->SetLongField(fileStatus, gFileStatusSizeFieldID, s.st_size);
146-
env->SetIntField(fileStatus, gFileStatusBlksizeFieldID, s.st_blksize);
147-
env->SetLongField(fileStatus, gFileStatusBlocksFieldID, s.st_blocks);
148-
env->SetLongField(fileStatus, gFileStatusAtimeFieldID, s.st_atime);
149-
env->SetLongField(fileStatus, gFileStatusMtimeFieldID, s.st_mtime);
150-
env->SetLongField(fileStatus, gFileStatusCtimeFieldID, s.st_ctime);
151-
}
152-
}
153-
154-
env->ReleaseStringUTFChars(path, pathStr);
155-
156-
return ret;
157-
}
158-
15984
static const JNINativeMethod methods[] = {
16085
{"setPermissions", "(Ljava/lang/String;III)I", (void*)android_os_FileUtils_setPermissions},
161-
{"getPermissions", "(Ljava/lang/String;[I)I", (void*)android_os_FileUtils_getPermissions},
16286
{"setUMask", "(I)I", (void*)android_os_FileUtils_setUMask},
16387
{"getFatVolumeId", "(Ljava/lang/String;)I", (void*)android_os_FileUtils_getFatVolumeId},
164-
{"getFileStatusNative", "(Ljava/lang/String;Landroid/os/FileUtils$FileStatus;)Z", (void*)android_os_FileUtils_getFileStatus},
16588
};
16689

16790
static const char* const kFileUtilsPathName = "android/os/FileUtils";
16891

16992
int register_android_os_FileUtils(JNIEnv* env)
17093
{
171-
jclass fileStatusClass = env->FindClass("android/os/FileUtils$FileStatus");
172-
LOG_FATAL_IF(fileStatusClass == NULL, "Unable to find class android.os.FileUtils$FileStatus");
173-
174-
gFileStatusDevFieldID = env->GetFieldID(fileStatusClass, "dev", "I");
175-
gFileStatusInoFieldID = env->GetFieldID(fileStatusClass, "ino", "I");
176-
gFileStatusModeFieldID = env->GetFieldID(fileStatusClass, "mode", "I");
177-
gFileStatusNlinkFieldID = env->GetFieldID(fileStatusClass, "nlink", "I");
178-
gFileStatusUidFieldID = env->GetFieldID(fileStatusClass, "uid", "I");
179-
gFileStatusGidFieldID = env->GetFieldID(fileStatusClass, "gid", "I");
180-
gFileStatusSizeFieldID = env->GetFieldID(fileStatusClass, "size", "J");
181-
gFileStatusBlksizeFieldID = env->GetFieldID(fileStatusClass, "blksize", "I");
182-
gFileStatusBlocksFieldID = env->GetFieldID(fileStatusClass, "blocks", "J");
183-
gFileStatusAtimeFieldID = env->GetFieldID(fileStatusClass, "atime", "J");
184-
gFileStatusMtimeFieldID = env->GetFieldID(fileStatusClass, "mtime", "J");
185-
gFileStatusCtimeFieldID = env->GetFieldID(fileStatusClass, "ctime", "J");
186-
18794
return AndroidRuntime::registerNativeMethods(
18895
env, kFileUtilsPathName,
18996
methods, NELEM(methods));

‎core/tests/coretests/src/android/os/FileUtilsTest.java‎

Lines changed: 1 addition & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,13 @@
1717
package android.os;
1818

1919
import android.content.Context;
20-
import android.os.FileUtils;
21-
import android.os.FileUtils.FileStatus;
2220
import android.test.AndroidTestCase;
23-
import android.test.suitebuilder.annotation.LargeTest;
2421
import android.test.suitebuilder.annotation.MediumTest;
25-
import android.test.suitebuilder.annotation.SmallTest;
2622

2723
import java.io.ByteArrayInputStream;
2824
import java.io.File;
29-
import java.io.FileWriter;
30-
import java.io.FileNotFoundException;
3125
import java.io.FileOutputStream;
32-
import java.io.IOException;
33-
34-
import junit.framework.Assert;
26+
import java.io.FileWriter;
3527

3628
public class FileUtilsTest extends AndroidTestCase {
3729
private static final String TEST_DATA =
@@ -60,60 +52,6 @@ protected void tearDown() throws Exception {
6052
if (mCopyFile.exists()) mCopyFile.delete();
6153
}
6254

63-
@LargeTest
64-
public void testGetFileStatus() {
65-
final byte[] MAGIC = { 0xB, 0xE, 0x0, 0x5 };
66-
67-
try {
68-
// truncate test file and write MAGIC (4 bytes) to it.
69-
FileOutputStream os = new FileOutputStream(mTestFile, false);
70-
os.write(MAGIC, 0, 4);
71-
os.flush();
72-
os.close();
73-
} catch (FileNotFoundException e) {
74-
Assert.fail("File was removed durning test" + e);
75-
} catch (IOException e) {
76-
Assert.fail("Unexpected IOException: " + e);
77-
}
78-
79-
Assert.assertTrue(mTestFile.exists());
80-
Assert.assertTrue(FileUtils.getFileStatus(mTestFile.getPath(), null));
81-
82-
FileStatus status1 = new FileStatus();
83-
FileUtils.getFileStatus(mTestFile.getPath(), status1);
84-
85-
Assert.assertEquals(4, status1.size);
86-
87-
// Sleep for at least one second so that the modification time will be different.
88-
try {
89-
Thread.sleep(1000);
90-
} catch (InterruptedException e) {
91-
}
92-
93-
try {
94-
// append so we don't change the creation time.
95-
FileOutputStream os = new FileOutputStream(mTestFile, true);
96-
os.write(MAGIC, 0, 4);
97-
os.flush();
98-
os.close();
99-
} catch (FileNotFoundException e) {
100-
Assert.fail("File was removed durning test" + e);
101-
} catch (IOException e) {
102-
Assert.fail("Unexpected IOException: " + e);
103-
}
104-
105-
FileStatus status2 = new FileStatus();
106-
FileUtils.getFileStatus(mTestFile.getPath(), status2);
107-
108-
Assert.assertEquals(8, status2.size);
109-
Assert.assertTrue(status2.mtime > status1.mtime);
110-
111-
mTestFile.delete();
112-
113-
Assert.assertFalse(mTestFile.exists());
114-
Assert.assertFalse(FileUtils.getFileStatus(mTestFile.getPath(), null));
115-
}
116-
11755
// TODO: test setPermissions(), getPermissions()
11856

11957
@MediumTest

0 commit comments

Comments
 (0)