Write mkdirs in more idiomatic C++ style.
~ Rewrote mkdirs to be in C++ style.
~ Replaced adb_dir{start,stop} with std::string params and (r)find.
+ Added test for mkdirs.
Also make base/test_utils.h public and support temporary directories
as well as files.
Change-Id: I6fcbdc5e0099f3359d3aac6b00c436f250ca1329
diff --git a/adb/adb_io_test.cpp b/adb/adb_io_test.cpp
index 8fd5cbf..0ae21db 100644
--- a/adb/adb_io_test.cpp
+++ b/adb/adb_io_test.cpp
@@ -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";
diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp
index 6fa6c2e..42191c6 100644
--- a/adb/adb_utils.cpp
+++ b/adb/adb_utils.cpp
@@ -72,24 +72,13 @@
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) {
diff --git a/adb/adb_utils.h b/adb/adb_utils.h
index 673aaac..64cbd9d 100644
--- a/adb/adb_utils.h
+++ b/adb/adb_utils.h
@@ -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);
diff --git a/adb/adb_utils_test.cpp b/adb/adb_utils_test.cpp
index 7aa610a..20dba27 100644
--- a/adb/adb_utils_test.cpp
+++ b/adb/adb_utils_test.cpp
@@ -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 @@
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/"));
+}
diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index d54faec..b0bcc88 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -859,7 +859,7 @@
// 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 @@
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) {
diff --git a/adb/file_sync_client.cpp b/adb/file_sync_client.cpp
index 49d42a3..1f4e95d 100644
--- a/adb/file_sync_client.cpp
+++ b/adb/file_sync_client.cpp
@@ -746,12 +746,9 @@
/* 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 @@
/* 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;
diff --git a/adb/file_sync_service.cpp b/adb/file_sync_service.cpp
index 2067836..94401f3 100644
--- a/adb/file_sync_service.cpp
+++ b/adb/file_sync_service.cpp
@@ -41,40 +41,31 @@
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 @@
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 @@
ret = symlink(buffer, path);
if(ret && errno == ENOENT) {
- if(mkdirs(path) != 0) {
+ if (!secure_mkdirs(path)) {
fail_errno(s);
return -1;
}
diff --git a/adb/sysdeps.h b/adb/sysdeps.h
index 729bbcb..3a3ffda 100644
--- a/adb/sysdeps.h
+++ b/adb/sysdeps.h
@@ -54,6 +54,8 @@
#include <windows.h>
#include <ws2tcpip.h>
+#include <string>
+
#include "fdevent.h"
#define OS_PATH_SEPARATOR '\\'
@@ -120,7 +122,7 @@
#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 @@
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 @@
#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 @@
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__ 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] == '/';
}
diff --git a/base/Android.mk b/base/Android.mk
index 7bd317b..4e135f6 100644
--- a/base/Android.mk
+++ b/base/Android.mk
@@ -21,6 +21,7 @@
logging.cpp \
stringprintf.cpp \
strings.cpp \
+ test_utils.cpp \
libbase_test_src_files := \
file_test.cpp \
@@ -28,7 +29,6 @@
stringprintf_test.cpp \
strings_test.cpp \
test_main.cpp \
- test_utils.cpp \
libbase_cppflags := \
-Wall \
diff --git a/base/file_test.cpp b/base/file_test.cpp
index b138094..4056684 100644
--- a/base/file_test.cpp
+++ b/base/file_test.cpp
@@ -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, 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, 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 @@
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);
}
diff --git a/base/test_utils.h b/base/include/base/test_utils.h
similarity index 68%
rename from base/test_utils.h
rename to base/include/base/test_utils.h
index 132d3a7..83f0f1c 100644
--- a/base/test_utils.h
+++ b/base/include/base/test_utils.h
@@ -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
diff --git a/base/logging_test.cpp b/base/logging_test.cpp
index c91857a..1a92c94 100644
--- a/base/logging_test.cpp
+++ b/base/logging_test.cpp
@@ -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>
diff --git a/base/test_utils.cpp b/base/test_utils.cpp
index 0517bc7..dceb8b7 100644
--- a/base/test_utils.cpp
+++ b/base/test_utils.cpp
@@ -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