Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c63cb070 authored by liwugang's avatar liwugang Committed by Elliott Hughes
Browse files

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: default avatarliwugang <liwugang@xiaomi.com>
parent 09738138

base/file.cpp

+8 −2
Original line number Diff line number Diff line
@@ -199,17 +199,23 @@ bool WriteFully(int fd, const void* data, size_t byte_count) { 
bool RemoveFileIfExists(const std::string& path, std::string* err) {

  struct stat st;

#if defined(_WIN32)

  //TODO: Windows version can't handle symbol link correctly.

  // TODO: Windows version can't handle symbolic links correctly.

  int result = stat(path.c_str(), &st);

  bool file_type_removable = (result == 0 && S_ISREG(st.st_mode));

#else

  int result = lstat(path.c_str(), &st);

  bool file_type_removable = (result == 0 && (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)));

#endif

  if (result == -1) {

    if (errno == ENOENT || errno == ENOTDIR) return true;

    if (err != nullptr) *err = strerror(errno);

    return false;

  }



  if (result == 0) {

    if (!file_type_removable) {

      if (err != nullptr) {

        *err = "is not a regular or symbol link file";

        *err = "is not a regular file or symbolic link";

      }

      return false;

    }

base/file_test.cpp

+40 −2
Original line number Diff line number Diff line
@@ -26,6 +26,10 @@ 


#include "android-base/test_utils.h"



#if !defined(_WIN32)

#include <pwd.h>

#endif



TEST(file, ReadFileToString_ENOENT) {

  std::string s("hello");

  errno = 0;
@@ -115,7 +119,7 @@ TEST(file, WriteFully) { 
  ASSERT_FALSE(android::base::ReadFully(tf.fd, &s[0], s.size()));

}



TEST(file, RemoveFileIfExist) {

TEST(file, RemoveFileIfExists) {

  TemporaryFile tf;

  ASSERT_TRUE(tf.fd != -1);

  close(tf.fd);
@@ -126,9 +130,43 @@ TEST(file, RemoveFileIfExist) { 
  TemporaryDir td;

  ASSERT_FALSE(android::base::RemoveFileIfExists(td.path));

  ASSERT_FALSE(android::base::RemoveFileIfExists(td.path, &err));

  ASSERT_EQ("is not a regular or symbol link file", err);

  ASSERT_EQ("is not a regular file or symbolic link", err);

}



TEST(file, RemoveFileIfExists_ENOTDIR) {

  TemporaryFile tf;

  close(tf.fd);

  tf.fd = -1;

  std::string err{"xxx"};

  ASSERT_TRUE(android::base::RemoveFileIfExists(std::string{tf.path} + "/abc", &err));

  ASSERT_EQ("xxx", err);

}



#if !defined(_WIN32)

TEST(file, RemoveFileIfExists_EACCES) {

  // EACCES -- one of the directories in the path has no search permission

  // root can bypass permission restrictions, so drop root.

  if (getuid() == 0) {

    passwd* shell = getpwnam("shell");

    setgid(shell->pw_gid);

    setuid(shell->pw_uid);

  }



  TemporaryDir td;

  TemporaryFile tf(td.path);

  close(tf.fd);

  tf.fd = -1;

  std::string err{"xxx"};

  // Remove dir's search permission.

  ASSERT_TRUE(chmod(td.path, S_IRUSR | S_IWUSR) == 0);

  ASSERT_FALSE(android::base::RemoveFileIfExists(tf.path, &err));

  ASSERT_EQ("Permission denied", err);

  // Set dir's search permission again.

  ASSERT_TRUE(chmod(td.path, S_IRWXU) == 0);

  ASSERT_TRUE(android::base::RemoveFileIfExists(tf.path, &err));

}

#endif



TEST(file, Readlink) {

#if !defined(_WIN32)

  // Linux doesn't allow empty symbolic links.