summaryrefslogtreecommitdiffstats
path: root/base
diff options
context:
space:
mode:
authorliwugang2018-07-11 00:24:49 -0500
committerElliott Hughes2018-07-12 19:35:17 -0500
commitc63cb070633864fe5424d8a2b6e44f0fd19ef08f (patch)
tree97067b15cdb73ef22653c45befa40e9222e2cf16 /base
parent097381382bb49547b847c90e0471694d84ce68f6 (diff)
downloadplatform-system-core-c63cb070633864fe5424d8a2b6e44f0fd19ef08f.tar.gz
platform-system-core-c63cb070633864fe5424d8a2b6e44f0fd19ef08f.tar.xz
platform-system-core-c63cb070633864fe5424d8a2b6e44f0fd19ef08f.zip
libbase: return different result depend on the errno
In the RemoveFileIfExists it always return true even if error appeared when using stat function. It should distinguish different error. Such as ENOENT and ENOTDIR we exactly know the file does not exist. But EACCES(current user has not all search permission in the file path) and other errors appeared we can't know whether file exits. So we should return false indicate there are some error appeared. Test: ran unit tests Change-Id: I75788bf0621040812413d52596b5effb628fd0b1 Signed-off-by: liwugang <liwugang@xiaomi.com>
Diffstat (limited to 'base')
-rw-r--r--base/file.cpp10
-rw-r--r--base/file_test.cpp42
2 files changed, 48 insertions, 4 deletions
diff --git a/base/file.cpp b/base/file.cpp
index 2f697a1cc..d6fe753d1 100644
--- a/base/file.cpp
+++ b/base/file.cpp
@@ -199,17 +199,23 @@ bool WriteFully(int fd, const void* data, size_t byte_count) {
199bool RemoveFileIfExists(const std::string& path, std::string* err) { 199bool RemoveFileIfExists(const std::string& path, std::string* err) {
200 struct stat st; 200 struct stat st;
201#if defined(_WIN32) 201#if defined(_WIN32)
202 //TODO: Windows version can't handle symbol link correctly. 202 // TODO: Windows version can't handle symbolic links correctly.
203 int result = stat(path.c_str(), &st); 203 int result = stat(path.c_str(), &st);
204 bool file_type_removable = (result == 0 && S_ISREG(st.st_mode)); 204 bool file_type_removable = (result == 0 && S_ISREG(st.st_mode));
205#else 205#else
206 int result = lstat(path.c_str(), &st); 206 int result = lstat(path.c_str(), &st);
207 bool file_type_removable = (result == 0 && (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode))); 207 bool file_type_removable = (result == 0 && (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)));
208#endif 208#endif
209 if (result == -1) {
210 if (errno == ENOENT || errno == ENOTDIR) return true;
211 if (err != nullptr) *err = strerror(errno);
212 return false;
213 }
214
209 if (result == 0) { 215 if (result == 0) {
210 if (!file_type_removable) { 216 if (!file_type_removable) {
211 if (err != nullptr) { 217 if (err != nullptr) {
212 *err = "is not a regular or symbol link file"; 218 *err = "is not a regular file or symbolic link";
213 } 219 }
214 return false; 220 return false;
215 } 221 }
diff --git a/base/file_test.cpp b/base/file_test.cpp
index 02b431de7..67946523c 100644
--- a/base/file_test.cpp
+++ b/base/file_test.cpp
@@ -26,6 +26,10 @@
26 26
27#include "android-base/test_utils.h" 27#include "android-base/test_utils.h"
28 28
29#if !defined(_WIN32)
30#include <pwd.h>
31#endif
32
29TEST(file, ReadFileToString_ENOENT) { 33TEST(file, ReadFileToString_ENOENT) {
30 std::string s("hello"); 34 std::string s("hello");
31 errno = 0; 35 errno = 0;
@@ -115,7 +119,7 @@ TEST(file, WriteFully) {
115 ASSERT_FALSE(android::base::ReadFully(tf.fd, &s[0], s.size())); 119 ASSERT_FALSE(android::base::ReadFully(tf.fd, &s[0], s.size()));
116} 120}
117 121
118TEST(file, RemoveFileIfExist) { 122TEST(file, RemoveFileIfExists) {
119 TemporaryFile tf; 123 TemporaryFile tf;
120 ASSERT_TRUE(tf.fd != -1); 124 ASSERT_TRUE(tf.fd != -1);
121 close(tf.fd); 125 close(tf.fd);
@@ -126,9 +130,43 @@ TEST(file, RemoveFileIfExist) {
126 TemporaryDir td; 130 TemporaryDir td;
127 ASSERT_FALSE(android::base::RemoveFileIfExists(td.path)); 131 ASSERT_FALSE(android::base::RemoveFileIfExists(td.path));
128 ASSERT_FALSE(android::base::RemoveFileIfExists(td.path, &err)); 132 ASSERT_FALSE(android::base::RemoveFileIfExists(td.path, &err));
129 ASSERT_EQ("is not a regular or symbol link file", err); 133 ASSERT_EQ("is not a regular file or symbolic link", err);
130} 134}
131 135
136TEST(file, RemoveFileIfExists_ENOTDIR) {
137 TemporaryFile tf;
138 close(tf.fd);
139 tf.fd = -1;
140 std::string err{"xxx"};
141 ASSERT_TRUE(android::base::RemoveFileIfExists(std::string{tf.path} + "/abc", &err));
142 ASSERT_EQ("xxx", err);
143}
144
145#if !defined(_WIN32)
146TEST(file, RemoveFileIfExists_EACCES) {
147 // EACCES -- one of the directories in the path has no search permission
148 // root can bypass permission restrictions, so drop root.
149 if (getuid() == 0) {
150 passwd* shell = getpwnam("shell");
151 setgid(shell->pw_gid);
152 setuid(shell->pw_uid);
153 }
154
155 TemporaryDir td;
156 TemporaryFile tf(td.path);
157 close(tf.fd);
158 tf.fd = -1;
159 std::string err{"xxx"};
160 // Remove dir's search permission.
161 ASSERT_TRUE(chmod(td.path, S_IRUSR | S_IWUSR) == 0);
162 ASSERT_FALSE(android::base::RemoveFileIfExists(tf.path, &err));
163 ASSERT_EQ("Permission denied", err);
164 // Set dir's search permission again.
165 ASSERT_TRUE(chmod(td.path, S_IRWXU) == 0);
166 ASSERT_TRUE(android::base::RemoveFileIfExists(tf.path, &err));
167}
168#endif
169
132TEST(file, Readlink) { 170TEST(file, Readlink) {
133#if !defined(_WIN32) 171#if !defined(_WIN32)
134 // Linux doesn't allow empty symbolic links. 172 // Linux doesn't allow empty symbolic links.