Refactor file_store to use smart pointers
Bug: b/119276649 Merge of http://go/wvgerrit/68903 Test: Android + Linux unit tests The FileSystem interface as it exists expects an Open for a file and then a Close when finished. However, the Close doesn't delete the file itself and depending on the platform, the underlying impl_ as well, leading to a memory leak. To fix this leak as well as harden against future memory issues, this change refactors the interface to shift away from raw pointers and towards smart pointers. Change-Id: I7a7132ea95cd3775796a540f510b698f4f27dd24
This commit is contained in:
@@ -12,6 +12,7 @@
|
||||
#include <sys/stat.h>
|
||||
#include <sys/types.h>
|
||||
#include <unistd.h>
|
||||
#include <cstring>
|
||||
|
||||
#include "file_store.h"
|
||||
#include "log.h"
|
||||
@@ -28,7 +29,7 @@ bool FileUtils::Exists(const std::string& path) {
|
||||
struct stat buf;
|
||||
int res = stat(path.c_str(), &buf) == 0;
|
||||
if (!res) {
|
||||
LOGV("File::Exists: stat failed: %d", errno);
|
||||
LOGV("File::Exists: stat failed: %d, %s", errno, strerror(errno));
|
||||
}
|
||||
return res;
|
||||
}
|
||||
@@ -53,7 +54,7 @@ bool FileUtils::Remove(const std::string& path) {
|
||||
closedir(dir);
|
||||
}
|
||||
if (rmdir(path.c_str())) {
|
||||
LOGW("File::Remove: rmdir failed: %d", errno);
|
||||
LOGW("File::Remove: rmdir failed: %d, %s", errno, strerror(errno));
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
@@ -62,7 +63,7 @@ bool FileUtils::Remove(const std::string& path) {
|
||||
if (wildcard_pos == std::string::npos) {
|
||||
// Handle file deletion
|
||||
if (unlink(path.c_str()) && (errno != ENOENT)) {
|
||||
LOGW("File::Remove: unlink failed: %d", errno);
|
||||
LOGW("File::Remove: unlink failed: %d, %s", errno, strerror(errno));
|
||||
return false;
|
||||
}
|
||||
} else {
|
||||
@@ -106,19 +107,22 @@ bool FileUtils::Remove(const std::string& path) {
|
||||
bool FileUtils::Copy(const std::string& src, const std::string& dest) {
|
||||
struct stat stat_buf;
|
||||
if (stat(src.c_str(), &stat_buf)) {
|
||||
LOGV("File::Copy: file %s stat error: %d", src.c_str(), errno);
|
||||
LOGV("File::Copy: file %s stat error: %d, %s", src.c_str(), errno,
|
||||
strerror(errno));
|
||||
return false;
|
||||
}
|
||||
|
||||
int fd_src = open(src.c_str(), O_RDONLY);
|
||||
if (fd_src < 0) {
|
||||
LOGW("File::Copy: unable to open file %s: %d", src.c_str(), errno);
|
||||
LOGW("File::Copy: unable to open file %s: %d, %s", src.c_str(), errno,
|
||||
strerror(errno));
|
||||
return false;
|
||||
}
|
||||
|
||||
int fd_dest = open(dest.c_str(), O_WRONLY | O_CREAT, stat_buf.st_mode);
|
||||
if (fd_dest < 0) {
|
||||
LOGW("File::Copy: unable to open file %s: %d", dest.c_str(), errno);
|
||||
LOGW("File::Copy: unable to open file %s: %d, %s", dest.c_str(), errno,
|
||||
strerror(errno));
|
||||
close(fd_src);
|
||||
return false;
|
||||
}
|
||||
@@ -126,8 +130,8 @@ bool FileUtils::Copy(const std::string& src, const std::string& dest) {
|
||||
off_t offset = 0;
|
||||
bool status = true;
|
||||
if (sendfile(fd_dest, fd_src, &offset, stat_buf.st_size) < 0) {
|
||||
LOGV("File::Copy: unable to copy %s to %s: %d", src.c_str(), dest.c_str(),
|
||||
errno);
|
||||
LOGV("File::Copy: unable to copy %s to %s: %d, %s", src.c_str(),
|
||||
dest.c_str(), errno, strerror(errno));
|
||||
status = false;
|
||||
}
|
||||
|
||||
@@ -143,13 +147,15 @@ bool FileUtils::List(const std::string& path, std::vector<std::string>* files) {
|
||||
}
|
||||
|
||||
if (!FileUtils::Exists(path)) {
|
||||
LOGV("File::List: path %s does not exist: %d", path.c_str(), errno);
|
||||
LOGV("File::List: path %s does not exist: %d, %s", path.c_str(), errno,
|
||||
strerror(errno));
|
||||
return false;
|
||||
}
|
||||
|
||||
DIR* dir = opendir(path.c_str());
|
||||
if (dir == NULL) {
|
||||
LOGW("File::List: unable to open directory %s: %d", path.c_str(), errno);
|
||||
LOGW("File::List: unable to open directory %s: %d, %s", path.c_str(), errno,
|
||||
strerror(errno));
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -193,7 +199,8 @@ bool FileUtils::CreateDirectory(const std::string& path_in) {
|
||||
path[pos] = '\0';
|
||||
if (mkdir(path.c_str(), 0700) != 0) {
|
||||
if (errno != EEXIST) {
|
||||
LOGW("File::CreateDirectory: mkdir failed: %d\n", errno);
|
||||
LOGW("File::CreateDirectory: mkdir failed: %d, %s\n", errno,
|
||||
strerror(errno));
|
||||
return false;
|
||||
}
|
||||
}
|
||||
@@ -204,7 +211,8 @@ bool FileUtils::CreateDirectory(const std::string& path_in) {
|
||||
if (path[size - 1] != kDirectoryDelimiter) {
|
||||
if (mkdir(path.c_str(), 0700) != 0) {
|
||||
if (errno != EEXIST) {
|
||||
LOGW("File::CreateDirectory: mkdir failed: %d\n", errno);
|
||||
LOGW("File::CreateDirectory: mkdir failed: %d, %s\n", errno,
|
||||
strerror(errno));
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user