Merge "Write mkdirs in more idiomatic C++ style."

This commit is contained in:
Elliott Hughes 2015-07-30 22:56:01 +00:00 committed by Gerrit Code Review
commit 0cf93dc345
13 changed files with 139 additions and 131 deletions

View File

@ -28,30 +28,7 @@
#include <string>
#include "base/file.h"
class TemporaryFile {
public:
TemporaryFile() {
init("/data/local/tmp");
if (fd == -1) {
init("/tmp");
}
}
~TemporaryFile() {
close(fd);
unlink(filename);
}
int fd;
char filename[1024];
private:
void init(const char* tmp_dir) {
snprintf(filename, sizeof(filename), "%s/TemporaryFile-XXXXXX", tmp_dir);
fd = mkstemp(filename);
}
};
#include "base/test_utils.h"
TEST(io, ReadFdExactly_whole) {
const char expected[] = "Foobar";

View File

@ -72,24 +72,13 @@ std::string escape_arg(const std::string& s) {
return result;
}
int mkdirs(const std::string& path) {
// TODO: rewrite this function and merge it with the *other* mkdirs in adb.
std::unique_ptr<char> path_rw(strdup(path.c_str()));
int ret;
char* x = path_rw.get() + 1;
for(;;) {
x = const_cast<char*>(adb_dirstart(x));
if(x == 0) return 0;
*x = 0;
ret = adb_mkdir(path_rw.get(), 0775);
*x = OS_PATH_SEPARATOR;
if((ret < 0) && (errno != EEXIST)) {
return ret;
bool mkdirs(const std::string& path) {
for (size_t i = adb_dirstart(path, 1); i != std::string::npos; i = adb_dirstart(path, i + 1)) {
if (adb_mkdir(path.substr(0, i), 0775) == -1 && errno != EEXIST) {
return false;
}
x++;
}
return 0;
return true;
}
void dump_hex(const void* data, size_t byte_count) {

View File

@ -22,7 +22,7 @@
bool getcwd(std::string* cwd);
bool directory_exists(const std::string& path);
int mkdirs(const std::string& path);
bool mkdirs(const std::string& path);
std::string escape_arg(const std::string& s);

View File

@ -18,6 +18,13 @@
#include <gtest/gtest.h>
#include <stdlib.h>
#include <string.h>
#include "sysdeps.h"
#include <base/test_utils.h>
TEST(adb_utils, directory_exists) {
ASSERT_TRUE(directory_exists("/proc"));
ASSERT_FALSE(directory_exists("/proc/self")); // Symbolic link.
@ -132,3 +139,11 @@ TEST(adb_utils, parse_host_and_port) {
EXPECT_FALSE(parse_host_and_port("1.2.3.4:0", &canonical_address, &host, &port, &error));
EXPECT_FALSE(parse_host_and_port("1.2.3.4:65536", &canonical_address, &host, &port, &error));
}
TEST(adb_utils, mkdirs) {
TemporaryDir td;
EXPECT_TRUE(mkdirs(std::string(td.path) + "/dir/subdir/file"));
std::string file = std::string(td.path) + "/file";
adb_creat(file.c_str(), 0600);
EXPECT_FALSE(mkdirs(file + "/subdir/"));
}

View File

@ -859,7 +859,7 @@ static std::string find_product_out_path(const char* hint) {
// If there are any slashes in it, assume it's a relative path;
// make it absolute.
if (adb_dirstart(hint) != nullptr) {
if (adb_dirstart(hint) != std::string::npos) {
std::string cwd;
if (!getcwd(&cwd)) {
fprintf(stderr, "adb: getcwd failed: %s\n", strerror(errno));
@ -1467,15 +1467,15 @@ static int delete_file(TransportType transport, const char* serial, char* filena
return send_shell_command(transport, serial, cmd);
}
static const char* get_basename(const char* filename)
static const char* get_basename(const std::string& filename)
{
const char* basename = adb_dirstop(filename);
if (basename) {
basename++;
return basename;
size_t base = adb_dirstop(filename);
if (base != std::string::npos) {
++base;
} else {
return filename;
base = 0;
}
return filename.c_str() + base;
}
static int install_app(TransportType transport, const char* serial, int argc, const char** argv) {

View File

@ -746,12 +746,9 @@ int do_sync_push(const char *lpath, const char *rpath, int show_progress)
/* if we're copying a local file to a remote directory,
** we *really* want to copy to remotedir + "/" + localfilename
*/
const char *name = adb_dirstop(lpath);
if(name == 0) {
name = lpath;
} else {
name++;
}
size_t slash = adb_dirstop(lpath);
const char *name = (slash == std::string::npos) ? lpath : lpath + slash + 1;
int tmplen = strlen(name) + strlen(rpath) + 2;
char *tmp = reinterpret_cast<char*>(
malloc(strlen(name) + strlen(rpath) + 2));
@ -960,12 +957,9 @@ int do_sync_pull(const char *rpath, const char *lpath, int show_progress, int co
/* if we're copying a remote file to a local directory,
** we *really* want to copy to localdir + "/" + remotefilename
*/
const char *name = adb_dirstop(rpath);
if(name == 0) {
name = rpath;
} else {
name++;
}
size_t slash = adb_dirstop(rpath);
const char *name = (slash == std::string::npos) ? rpath : rpath + slash + 1;
int tmplen = strlen(name) + strlen(lpath) + 2;
char *tmp = reinterpret_cast<char*>(malloc(tmplen));
if(tmp == 0) return 1;

View File

@ -41,40 +41,31 @@ static bool should_use_fs_config(const char* path) {
strncmp("/oem/", path, strlen("/oem/")) == 0;
}
static int mkdirs(char *name)
{
int ret;
char *x = name + 1;
static bool secure_mkdirs(const std::string& path) {
uid_t uid = -1;
gid_t gid = -1;
unsigned int mode = 0775;
uint64_t cap = 0;
if(name[0] != '/') return -1;
if (path[0] != '/') return false;
for(;;) {
x = const_cast<char*>(adb_dirstart(x));
if(x == 0) return 0;
*x = 0;
if (should_use_fs_config(name)) {
fs_config(name, 1, &uid, &gid, &mode, &cap);
for (size_t i = adb_dirstart(path, 1); i != std::string::npos; i = adb_dirstart(path, i + 1)) {
std::string name(path.substr(0, i));
if (should_use_fs_config(name.c_str())) {
fs_config(name.c_str(), 1, &uid, &gid, &mode, &cap);
}
ret = adb_mkdir(name, mode);
if((ret < 0) && (errno != EEXIST)) {
D("mkdir(\"%s\") -> %s\n", name, strerror(errno));
*x = '/';
return ret;
} else if(ret == 0) {
ret = chown(name, uid, gid);
if (ret < 0) {
*x = '/';
return ret;
if (adb_mkdir(name.c_str(), mode) == -1) {
if (errno != EEXIST) {
return false;
}
selinux_android_restorecon(name, 0);
} else {
if (chown(name.c_str(), uid, gid) == -1) {
return false;
}
selinux_android_restorecon(name.c_str(), 0);
}
*x++ = '/';
}
return 0;
return true;
}
static int do_stat(int s, const char *path)
@ -182,7 +173,7 @@ static int handle_send_file(int s, char *path, uid_t uid,
fd = adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, mode);
if(fd < 0 && errno == ENOENT) {
if(mkdirs(path) != 0) {
if (!secure_mkdirs(path)) {
if(fail_errno(s))
return -1;
fd = -1;
@ -294,7 +285,7 @@ static int handle_send_link(int s, char *path, char *buffer)
ret = symlink(buffer, path);
if(ret && errno == ENOENT) {
if(mkdirs(path) != 0) {
if (!secure_mkdirs(path)) {
fail_errno(s);
return -1;
}

View File

@ -54,6 +54,8 @@
#include <windows.h>
#include <ws2tcpip.h>
#include <string>
#include "fdevent.h"
#define OS_PATH_SEPARATOR '\\'
@ -120,7 +122,7 @@ static __inline__ int adb_unlink(const char* path)
#undef unlink
#define unlink ___xxx_unlink
static __inline__ int adb_mkdir(const char* path, int mode)
static __inline__ int adb_mkdir(const std::string& path, int mode)
{
return _mkdir(path);
}
@ -234,27 +236,25 @@ static __inline__ void disable_tcp_nagle( int fd )
extern int adb_socketpair( int sv[2] );
static __inline__ char* adb_dirstart( const char* path )
{
char* p = strchr(path, '/');
char* p2 = strchr(path, '\\');
static __inline__ size_t adb_dirstart(const std::string& path, size_t pos = 0) {
size_t p = path.find('/', pos);
size_t p2 = path.find('\\', pos);
if ( !p )
if ( p == std::string::npos )
p = p2;
else if ( p2 && p2 > p )
else if ( p2 != std::string::npos && p2 > p )
p = p2;
return p;
}
static __inline__ const char* adb_dirstop( const char* path )
{
const char* p = strrchr(path, '/');
const char* p2 = strrchr(path, '\\');
static __inline__ size_t adb_dirstop(const std::string& path) {
size_t p = path.rfind('/');
size_t p2 = path.rfind('\\');
if ( !p )
if ( p == std::string::npos )
p = p2;
else if ( p2 && p2 > p )
else if ( p2 != std::string::npos && p2 > p )
p = p2;
return p;
@ -284,6 +284,8 @@ static __inline__ int adb_is_absolute_host_path( const char* path )
#include <string.h>
#include <unistd.h>
#include <string>
#define OS_PATH_SEPARATOR '/'
#define OS_PATH_SEPARATOR_STR "/"
#define ENV_PATH_SEPARATOR_STR ":"
@ -510,10 +512,11 @@ static __inline__ void adb_sleep_ms( int mseconds )
usleep( mseconds*1000 );
}
static __inline__ int adb_mkdir(const char* path, int mode)
static __inline__ int adb_mkdir(const std::string& path, int mode)
{
return mkdir(path, mode);
return mkdir(path.c_str(), mode);
}
#undef mkdir
#define mkdir ___xxx_mkdir
@ -521,18 +524,15 @@ static __inline__ void adb_sysdeps_init(void)
{
}
static __inline__ const char* adb_dirstart(const char* path)
{
return strchr(path, '/');
static __inline__ size_t adb_dirstart(const std::string& path, size_t pos = 0) {
return path.find('/', pos);
}
static __inline__ const char* adb_dirstop(const char* path)
{
return strrchr(path, '/');
static __inline__ size_t adb_dirstop(const std::string& path) {
return path.rfind('/');
}
static __inline__ int adb_is_absolute_host_path( const char* path )
{
static __inline__ int adb_is_absolute_host_path(const char* path) {
return path[0] == '/';
}

View File

@ -21,6 +21,7 @@ libbase_src_files := \
logging.cpp \
stringprintf.cpp \
strings.cpp \
test_utils.cpp \
libbase_test_src_files := \
file_test.cpp \
@ -28,7 +29,6 @@ libbase_test_src_files := \
stringprintf_test.cpp \
strings_test.cpp \
test_main.cpp \
test_utils.cpp \
libbase_cppflags := \
-Wall \

View File

@ -24,7 +24,7 @@
#include <string>
#include "test_utils.h"
#include "base/test_utils.h"
TEST(file, ReadFileToString_ENOENT) {
std::string s("hello");
@ -47,10 +47,10 @@ TEST(file, ReadFileToString_success) {
TEST(file, WriteStringToFile) {
TemporaryFile tf;
ASSERT_TRUE(tf.fd != -1);
ASSERT_TRUE(android::base::WriteStringToFile("abc", tf.filename))
ASSERT_TRUE(android::base::WriteStringToFile("abc", tf.path))
<< strerror(errno);
std::string s;
ASSERT_TRUE(android::base::ReadFileToString(tf.filename, &s))
ASSERT_TRUE(android::base::ReadFileToString(tf.path, &s))
<< strerror(errno);
EXPECT_EQ("abc", s);
}
@ -61,16 +61,16 @@ TEST(file, WriteStringToFile) {
TEST(file, WriteStringToFile2) {
TemporaryFile tf;
ASSERT_TRUE(tf.fd != -1);
ASSERT_TRUE(android::base::WriteStringToFile("abc", tf.filename, 0660,
ASSERT_TRUE(android::base::WriteStringToFile("abc", tf.path, 0660,
getuid(), getgid()))
<< strerror(errno);
struct stat sb;
ASSERT_EQ(0, stat(tf.filename, &sb));
ASSERT_EQ(0, stat(tf.path, &sb));
ASSERT_EQ(0660U, static_cast<unsigned int>(sb.st_mode & ~S_IFMT));
ASSERT_EQ(getuid(), sb.st_uid);
ASSERT_EQ(getgid(), sb.st_gid);
std::string s;
ASSERT_TRUE(android::base::ReadFileToString(tf.filename, &s))
ASSERT_TRUE(android::base::ReadFileToString(tf.path, &s))
<< strerror(errno);
EXPECT_EQ("abc", s);
}
@ -109,7 +109,7 @@ TEST(file, WriteFully) {
ASSERT_TRUE(tf.fd != -1);
ASSERT_TRUE(android::base::WriteFully(tf.fd, "abc", 3));
std::string s;
ASSERT_TRUE(android::base::ReadFileToString(tf.filename, &s))
ASSERT_TRUE(android::base::ReadFileToString(tf.path, &s))
<< strerror(errno);
EXPECT_EQ("abc", s);
}

View File

@ -17,16 +17,37 @@
#ifndef TEST_UTILS_H
#define TEST_UTILS_H
#include <string>
#include <base/macros.h>
class TemporaryFile {
public:
TemporaryFile();
~TemporaryFile();
int fd;
char filename[1024];
char path[1024];
private:
void init(const char* tmp_dir);
void init(const std::string& tmp_dir);
DISALLOW_COPY_AND_ASSIGN(TemporaryFile);
};
#if !defined(_WIN32)
class TemporaryDir {
public:
TemporaryDir();
~TemporaryDir();
char path[1024];
private:
bool init(const std::string& tmp_dir);
DISALLOW_COPY_AND_ASSIGN(TemporaryDir);
};
#endif
#endif // TEST_UTILS_H

View File

@ -21,7 +21,7 @@
#include "base/file.h"
#include "base/stringprintf.h"
#include "test_utils.h"
#include "base/test_utils.h"
#include <gtest/gtest.h>

View File

@ -14,7 +14,7 @@
* limitations under the License.
*/
#include "test_utils.h"
#include "base/test_utils.h"
#include <fcntl.h>
#include <stdio.h>
@ -26,34 +26,55 @@
#include <windows.h>
#endif
TemporaryFile::TemporaryFile() {
#include <string>
static std::string GetSystemTempDir() {
#if defined(__ANDROID__)
init("/data/local/tmp");
return "/data/local/tmp";
#elif defined(_WIN32)
char wd[MAX_PATH] = {};
_getcwd(wd, sizeof(wd));
init(wd);
return wd;
#else
init("/tmp");
return "/tmp";
#endif
}
TemporaryFile::TemporaryFile() {
init(GetSystemTempDir());
}
TemporaryFile::~TemporaryFile() {
close(fd);
unlink(filename);
unlink(path);
}
void TemporaryFile::init(const char* tmp_dir) {
snprintf(filename, sizeof(filename), "%s/TemporaryFile-XXXXXX", tmp_dir);
void TemporaryFile::init(const std::string& tmp_dir) {
snprintf(path, sizeof(path), "%s/TemporaryFile-XXXXXX", tmp_dir.c_str());
#if !defined(_WIN32)
fd = mkstemp(filename);
fd = mkstemp(path);
#else
// Windows doesn't have mkstemp, and tmpfile creates the file in the root
// directory, requiring root (?!) permissions. We have to settle for mktemp.
if (mktemp(filename) == nullptr) {
if (mktemp(path) == nullptr) {
abort();
}
fd = open(filename, O_RDWR | O_NOINHERIT | O_CREAT, _S_IREAD | _S_IWRITE);
fd = open(path, O_RDWR | O_NOINHERIT | O_CREAT, _S_IREAD | _S_IWRITE);
#endif
}
#if !defined(_WIN32)
TemporaryDir::TemporaryDir() {
init(GetSystemTempDir());
}
TemporaryDir::~TemporaryDir() {
rmdir(path);
}
bool TemporaryDir::init(const std::string& tmp_dir) {
snprintf(path, sizeof(path), "%s/TemporaryDir-XXXXXX", tmp_dir.c_str());
return (mkdtemp(path) != nullptr);
}
#endif