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_io_test.cpp b/adb_io_test.cpp
index 8fd5cbf..0ae21db 100644
--- a/adb_io_test.cpp
+++ b/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_utils.cpp b/adb_utils.cpp
index 6fa6c2e..42191c6 100644
--- a/adb_utils.cpp
+++ b/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_utils.h b/adb_utils.h
index 673aaac..64cbd9d 100644
--- a/adb_utils.h
+++ b/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_utils_test.cpp b/adb_utils_test.cpp
index 7aa610a..20dba27 100644
--- a/adb_utils_test.cpp
+++ b/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/commandline.cpp b/commandline.cpp
index d54faec..b0bcc88 100644
--- a/commandline.cpp
+++ b/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/file_sync_client.cpp b/file_sync_client.cpp
index 49d42a3..1f4e95d 100644
--- a/file_sync_client.cpp
+++ b/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/file_sync_service.cpp b/file_sync_service.cpp
index 2067836..94401f3 100644
--- a/file_sync_service.cpp
+++ b/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/sysdeps.h b/sysdeps.h
index 729bbcb..3a3ffda 100644
--- a/sysdeps.h
+++ b/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] == '/';
}