More adb cleanup.
This removes adb_dirstart and adb_dirstop. It also fixes a couple of memory
leaks by switching to std::string. This also fixes the bug in the previous
change --- mkdirs is given input like "/system/bin/sh" and only expected to
create "/system/bin". In a later change, we should remove mkdirs and only
expose the intended "unlink && mkdirs && create" functionality.
Change-Id: I30289dc1b3dff575cc1b158d993652178f587552
diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp
index 42191c6..3ba971d 100644
--- a/adb/adb_utils.cpp
+++ b/adb/adb_utils.cpp
@@ -72,15 +72,38 @@
return result;
}
-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) {
+std::string adb_basename(const std::string& path) {
+ size_t base = path.find_last_of(OS_PATH_SEPARATORS);
+ return (base != std::string::npos) ? path.substr(base + 1) : path;
+}
+
+static bool real_mkdirs(const std::string& path) {
+ std::vector<std::string> path_components = android::base::Split(path, OS_PATH_SEPARATOR_STR);
+ // TODO: all the callers do unlink && mkdirs && adb_creat ---
+ // that's probably the operation we should expose.
+ path_components.pop_back();
+ std::string partial_path;
+ for (const auto& path_component : path_components) {
+ if (partial_path.back() != OS_PATH_SEPARATOR) partial_path += OS_PATH_SEPARATOR;
+ partial_path += path_component;
+ if (adb_mkdir(partial_path.c_str(), 0775) == -1 && errno != EEXIST) {
return false;
}
}
return true;
}
+bool mkdirs(const std::string& path) {
+#if defined(_WIN32)
+ // Replace '/' with '\\' so we can share the code.
+ std::string clean_path = path;
+ std::replace(clean_path.begin(), clean_path.end(), '/', '\\');
+ return real_mkdirs(clean_path);
+#else
+ return real_mkdirs(path);
+#endif
+}
+
void dump_hex(const void* data, size_t byte_count) {
byte_count = std::min(byte_count, size_t(16));
diff --git a/adb/adb_utils.h b/adb/adb_utils.h
index 64cbd9d..30cd7a4 100644
--- a/adb/adb_utils.h
+++ b/adb/adb_utils.h
@@ -21,6 +21,7 @@
bool getcwd(std::string* cwd);
bool directory_exists(const std::string& path);
+std::string adb_basename(const std::string& path);
bool mkdirs(const std::string& path);
diff --git a/adb/adb_utils_test.cpp b/adb/adb_utils_test.cpp
index 20dba27..309ac02 100644
--- a/adb/adb_utils_test.cpp
+++ b/adb/adb_utils_test.cpp
@@ -58,6 +58,11 @@
ASSERT_EQ(R"('abc)')", escape_arg("abc)"));
}
+TEST(adb_utils, adb_basename) {
+ EXPECT_EQ("sh", adb_basename("/system/bin/sh"));
+ EXPECT_EQ("sh", adb_basename("sh"));
+}
+
TEST(adb_utils, parse_host_and_port) {
std::string canonical_address;
std::string host;
@@ -142,8 +147,8 @@
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/"));
+ std::string path = std::string(td.path) + "/dir/subdir/file";
+ EXPECT_TRUE(mkdirs(path));
+ EXPECT_NE(-1, adb_creat(path.c_str(), 0600));
+ EXPECT_FALSE(mkdirs(path + "/subdir/"));
}
diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index b0bcc88..5e89f95 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -847,25 +847,25 @@
* Given <hint>, try to construct an absolute path to the
* ANDROID_PRODUCT_OUT dir.
*/
-static std::string find_product_out_path(const char* hint) {
- if (hint == NULL || hint[0] == '\0') {
+static std::string find_product_out_path(const std::string& hint) {
+ if (hint.empty()) {
return "";
}
// If it's already absolute, don't bother doing any work.
- if (adb_is_absolute_host_path(hint)) {
+ if (adb_is_absolute_host_path(hint.c_str())) {
return hint;
}
// If there are any slashes in it, assume it's a relative path;
// make it absolute.
- if (adb_dirstart(hint) != std::string::npos) {
+ if (hint.find_first_of(OS_PATH_SEPARATORS) != std::string::npos) {
std::string cwd;
if (!getcwd(&cwd)) {
fprintf(stderr, "adb: getcwd failed: %s\n", strerror(errno));
return "";
}
- return android::base::StringPrintf("%s%s%s", cwd.c_str(), OS_PATH_SEPARATOR_STR, hint);
+ return android::base::StringPrintf("%s%c%s", cwd.c_str(), OS_PATH_SEPARATOR, hint.c_str());
}
// It's a string without any slashes. Try to do something with it.
@@ -889,7 +889,7 @@
path += hint;
if (!directory_exists(path)) {
fprintf(stderr, "adb: Couldn't find a product dir based on -p %s; "
- "\"%s\" doesn't exist\n", hint, path.c_str());
+ "\"%s\" doesn't exist\n", hint.c_str(), path.c_str());
return "";
}
return path;
@@ -999,7 +999,7 @@
} else {
product = argv[0] + 2;
}
- gProductOutPath = find_product_out_path(product);
+ if (product) gProductOutPath = find_product_out_path(product);
if (gProductOutPath.empty()) {
fprintf(stderr, "adb: could not resolve \"-p %s\"\n", product);
return usage();
@@ -1462,22 +1462,11 @@
return pm_command(transport, serial, argc, argv);
}
-static int delete_file(TransportType transport, const char* serial, char* filename) {
+static int delete_file(TransportType transport, const char* serial, const std::string& filename) {
std::string cmd = "shell:rm -f " + escape_arg(filename);
return send_shell_command(transport, serial, cmd);
}
-static const char* get_basename(const std::string& filename)
-{
- size_t base = adb_dirstop(filename);
- if (base != std::string::npos) {
- ++base;
- } else {
- base = 0;
- }
- return filename.c_str() + base;
-}
-
static int install_app(TransportType transport, const char* serial, int argc, const char** argv) {
static const char *const DATA_DEST = "/data/local/tmp/%s";
static const char *const SD_DEST = "/sdcard/tmp/%s";
@@ -1514,13 +1503,12 @@
}
const char* apk_file = argv[last_apk];
- char apk_dest[PATH_MAX];
- snprintf(apk_dest, sizeof apk_dest, where, get_basename(apk_file));
- int err = do_sync_push(apk_file, apk_dest, 0 /* no show progress */);
+ std::string apk_dest = android::base::StringPrintf(where, adb_basename(apk_file).c_str());
+ int err = do_sync_push(apk_file, apk_dest.c_str(), 0 /* no show progress */);
if (err) {
goto cleanup_apk;
} else {
- argv[last_apk] = apk_dest; /* destination name, not source location */
+ argv[last_apk] = apk_dest.c_str(); /* destination name, not source location */
}
err = pm_command(transport, serial, argc, argv);
@@ -1604,7 +1592,7 @@
std::string cmd = android::base::StringPrintf(
"exec:pm install-write -S %" PRIu64 " %d %d_%s -",
- static_cast<uint64_t>(sb.st_size), session_id, i, get_basename(file));
+ static_cast<uint64_t>(sb.st_size), session_id, i, adb_basename(file).c_str());
int localFd = adb_open(file, O_RDONLY);
if (localFd < 0) {
diff --git a/adb/file_sync_client.cpp b/adb/file_sync_client.cpp
index 1f4e95d..da80013 100644
--- a/adb/file_sync_client.cpp
+++ b/adb/file_sync_client.cpp
@@ -35,6 +35,8 @@
#include "adb_utils.h"
#include "file_sync_service.h"
+#include <base/stringprintf.h>
+
static unsigned long long total_bytes;
static long long start_time;
@@ -92,36 +94,30 @@
typedef void (*sync_ls_cb)(unsigned mode, unsigned size, unsigned time, const char *name, void *cookie);
static int sync_ls(int fd, const char* path, sync_ls_cb func, void* cookie) {
+ int len = strlen(path);
+ if (len > 1024) goto fail;
+
syncmsg msg;
- char buf[257];
- int len;
-
- len = strlen(path);
- if(len > 1024) goto fail;
-
msg.req.id = ID_LIST;
msg.req.namelen = htoll(len);
- if(!WriteFdExactly(fd, &msg.req, sizeof(msg.req)) ||
- !WriteFdExactly(fd, path, len)) {
+ if (!WriteFdExactly(fd, &msg.req, sizeof(msg.req)) || !WriteFdExactly(fd, path, len)) {
goto fail;
}
- for(;;) {
- if(!ReadFdExactly(fd, &msg.dent, sizeof(msg.dent))) break;
- if(msg.dent.id == ID_DONE) return 0;
- if(msg.dent.id != ID_DENT) break;
+ for (;;) {
+ if (!ReadFdExactly(fd, &msg.dent, sizeof(msg.dent))) break;
+ if (msg.dent.id == ID_DONE) return 0;
+ if (msg.dent.id != ID_DENT) break;
len = ltohl(msg.dent.namelen);
- if(len > 256) break;
+ if (len > 256) break;
- if(!ReadFdExactly(fd, buf, len)) break;
+ char buf[257];
+ if (!ReadFdExactly(fd, buf, len)) break;
buf[len] = 0;
- func(ltohl(msg.dent.mode),
- ltohl(msg.dent.size),
- ltohl(msg.dent.time),
- buf, cookie);
+ func(ltohl(msg.dent.mode), ltohl(msg.dent.size), ltohl(msg.dent.time), buf, cookie);
}
fail:
@@ -220,7 +216,7 @@
return 0;
}
-static int write_data_file(int fd, const char *path, syncsendbuf *sbuf, int show_progress)
+static int write_data_file(int fd, const char *path, syncsendbuf *sbuf, bool show_progress)
{
int lfd, err = 0;
unsigned long long size = 0;
@@ -274,7 +270,7 @@
}
static int write_data_buffer(int fd, char* file_buffer, int size, syncsendbuf *sbuf,
- int show_progress)
+ bool show_progress)
{
int err = 0;
int total = 0;
@@ -308,10 +304,8 @@
#else
static int write_data_link(int fd, const char *path, syncsendbuf *sbuf)
{
- int len, ret;
-
- len = readlink(path, sbuf->data, SYNC_DATA_MAX-1);
- if(len < 0) {
+ int len = readlink(path, sbuf->data, SYNC_DATA_MAX-1);
+ if (len < 0) {
fprintf(stderr, "error reading link '%s': %s\n", path, strerror(errno));
return -1;
}
@@ -320,9 +314,9 @@
sbuf->size = htoll(len + 1);
sbuf->id = ID_DATA;
- ret = !WriteFdExactly(fd, sbuf, sizeof(unsigned) * 2 + len + 1);
- if(ret)
+ if (!WriteFdExactly(fd, sbuf, sizeof(unsigned) * 2 + len + 1)) {
return -1;
+ }
total_bytes += len + 1;
@@ -331,7 +325,7 @@
#endif
static int sync_send(int fd, const char *lpath, const char *rpath,
- unsigned mtime, mode_t mode, int show_progress)
+ unsigned mtime, mode_t mode, bool show_progress)
{
syncmsg msg;
int len, r;
@@ -396,7 +390,7 @@
return -1;
}
-static int sync_recv(int fd, const char* rpath, const char* lpath, int show_progress) {
+static int sync_recv(int fd, const char* rpath, const char* lpath, bool show_progress) {
syncmsg msg;
int len;
int lfd = -1;
@@ -523,12 +517,12 @@
return 1;
}
- if(sync_ls(fd, path, do_sync_ls_cb, 0)) {
+ if (sync_ls(fd, path, do_sync_ls_cb, 0)) {
return 1;
- } else {
- sync_quit(fd);
- return 0;
}
+
+ sync_quit(fd);
+ return 0;
}
struct copyinfo
@@ -712,11 +706,7 @@
}
-int do_sync_push(const char *lpath, const char *rpath, int show_progress)
-{
- struct stat st;
- unsigned mode;
-
+int do_sync_push(const char* lpath, const char* rpath, bool show_progress) {
std::string error;
int fd = adb_connect("sync:", &error);
if (fd < 0) {
@@ -724,48 +714,38 @@
return 1;
}
- if(stat(lpath, &st)) {
+ struct stat st;
+ if (stat(lpath, &st)) {
fprintf(stderr,"cannot stat '%s': %s\n", lpath, strerror(errno));
sync_quit(fd);
return 1;
}
- if(S_ISDIR(st.st_mode)) {
+ if (S_ISDIR(st.st_mode)) {
BEGIN();
- if(copy_local_dir_remote(fd, lpath, rpath, 0, 0)) {
+ if (copy_local_dir_remote(fd, lpath, rpath, 0, 0)) {
return 1;
- } else {
- END();
- sync_quit(fd);
}
} else {
- if(sync_readmode(fd, rpath, &mode)) {
+ unsigned mode;
+ if (sync_readmode(fd, rpath, &mode)) {
return 1;
}
- if((mode != 0) && S_ISDIR(mode)) {
- /* if we're copying a local file to a remote directory,
- ** we *really* want to copy to remotedir + "/" + localfilename
- */
- 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));
- if(tmp == 0) return 1;
- snprintf(tmp, tmplen, "%s/%s", rpath, name);
- rpath = tmp;
+ std::string path_holder;
+ if ((mode != 0) && S_ISDIR(mode)) {
+ // If we're copying a local file to a remote directory,
+ // we really want to copy to remote_dir + "/" + local_filename.
+ path_holder = android::base::StringPrintf("%s/%s", rpath, adb_basename(lpath).c_str());
+ rpath = path_holder.c_str();
}
BEGIN();
- if(sync_send(fd, lpath, rpath, st.st_mtime, st.st_mode, show_progress)) {
+ if (sync_send(fd, lpath, rpath, st.st_mtime, st.st_mode, show_progress)) {
return 1;
- } else {
- END();
- sync_quit(fd);
- return 0;
}
}
+ END();
+ sync_quit(fd);
return 0;
}
@@ -931,11 +911,7 @@
return ret;
}
-int do_sync_pull(const char *rpath, const char *lpath, int show_progress, int copy_attrs)
-{
- unsigned mode, time;
- struct stat st;
-
+int do_sync_pull(const char* rpath, const char* lpath, bool show_progress, int copy_attrs) {
std::string error;
int fd = adb_connect("sync:", &error);
if (fd < 0) {
@@ -943,53 +919,46 @@
return 1;
}
- if(sync_readtime(fd, rpath, &time, &mode)) {
+ unsigned mode, time;
+ if (sync_readtime(fd, rpath, &time, &mode)) {
return 1;
}
- if(mode == 0) {
+ if (mode == 0) {
fprintf(stderr,"remote object '%s' does not exist\n", rpath);
return 1;
}
- if(S_ISREG(mode) || S_ISLNK(mode) || S_ISCHR(mode) || S_ISBLK(mode)) {
- if(stat(lpath, &st) == 0) {
- if(S_ISDIR(st.st_mode)) {
- /* if we're copying a remote file to a local directory,
- ** we *really* want to copy to localdir + "/" + remotefilename
- */
- 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;
- snprintf(tmp, tmplen, "%s/%s", lpath, name);
- lpath = tmp;
+ if (S_ISREG(mode) || S_ISLNK(mode) || S_ISCHR(mode) || S_ISBLK(mode)) {
+ std::string path_holder;
+ struct stat st;
+ if (stat(lpath, &st) == 0) {
+ if (S_ISDIR(st.st_mode)) {
+ // If we're copying a remote file to a local directory,
+ // we really want to copy to local_dir + "/" + basename(remote).
+ path_holder = android::base::StringPrintf("%s/%s", lpath, adb_basename(rpath).c_str());
+ lpath = path_holder.c_str();
}
}
BEGIN();
if (sync_recv(fd, rpath, lpath, show_progress)) {
return 1;
} else {
- if (copy_attrs && set_time_and_mode(lpath, time, mode))
+ if (copy_attrs && set_time_and_mode(lpath, time, mode)) {
return 1;
- END();
- sync_quit(fd);
- return 0;
+ }
}
} else if(S_ISDIR(mode)) {
BEGIN();
if (copy_remote_dir_local(fd, rpath, lpath, copy_attrs)) {
return 1;
- } else {
- END();
- sync_quit(fd);
- return 0;
}
} else {
fprintf(stderr,"remote object '%s' not a file or directory\n", rpath);
return 1;
}
+ END();
+ sync_quit(fd);
+ return 0;
}
int do_sync_sync(const std::string& lpath, const std::string& rpath, bool list_only)
@@ -1006,9 +975,8 @@
BEGIN();
if (copy_local_dir_remote(fd, lpath.c_str(), rpath.c_str(), 1, list_only)) {
return 1;
- } else {
- END();
- sync_quit(fd);
- return 0;
}
+ END();
+ sync_quit(fd);
+ return 0;
}
diff --git a/adb/file_sync_service.cpp b/adb/file_sync_service.cpp
index 94401f3..ea019f4 100644
--- a/adb/file_sync_service.cpp
+++ b/adb/file_sync_service.cpp
@@ -34,11 +34,13 @@
#include "adb_io.h"
#include "private/android_filesystem_config.h"
-static bool should_use_fs_config(const char* path) {
+#include <base/strings.h>
+
+static bool should_use_fs_config(const std::string& path) {
// TODO: use fs_config to configure permissions on /data.
- return strncmp("/system/", path, strlen("/system/")) == 0 ||
- strncmp("/vendor/", path, strlen("/vendor/")) == 0 ||
- strncmp("/oem/", path, strlen("/oem/")) == 0;
+ return android::base::StartsWith(path, "/system/") ||
+ android::base::StartsWith(path, "/vendor/") ||
+ android::base::StartsWith(path, "/oem/");
}
static bool secure_mkdirs(const std::string& path) {
@@ -49,20 +51,26 @@
if (path[0] != '/') return false;
- 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);
+ std::vector<std::string> path_components = android::base::Split(path, "/");
+ path_components.pop_back(); // For "/system/bin/sh", only create "/system/bin".
+
+ std::string partial_path;
+ for (auto& path_component : path_components) {
+ if (partial_path.back() != OS_PATH_SEPARATOR) partial_path += OS_PATH_SEPARATOR;
+ partial_path += path_component;
+
+ if (should_use_fs_config(partial_path)) {
+ fs_config(partial_path.c_str(), 1, &uid, &gid, &mode, &cap);
}
- if (adb_mkdir(name.c_str(), mode) == -1) {
+ if (adb_mkdir(partial_path.c_str(), mode) == -1) {
if (errno != EEXIST) {
return false;
}
} else {
- if (chown(name.c_str(), uid, gid) == -1) {
+ if (chown(partial_path.c_str(), uid, gid) == -1) {
return false;
}
- selinux_android_restorecon(name.c_str(), 0);
+ selinux_android_restorecon(partial_path.c_str(), 0);
}
}
return true;
@@ -90,26 +98,24 @@
static int do_list(int s, const char *path)
{
- DIR *d;
struct dirent *de;
struct stat st;
- syncmsg msg;
- int len;
char tmp[1024 + 256 + 1];
char *fname;
- len = strlen(path);
+ size_t len = strlen(path);
memcpy(tmp, path, len);
tmp[len] = '/';
fname = tmp + len + 1;
+ syncmsg msg;
msg.dent.id = ID_DENT;
- d = opendir(path);
- if(d == 0) goto done;
+ std::unique_ptr<DIR, int(*)(DIR*)> d(opendir(path), closedir);
+ if (!d) goto done;
- while((de = readdir(d))) {
+ while ((de = readdir(d.get()))) {
int len = strlen(de->d_name);
/* not supposed to be possible, but
@@ -125,14 +131,11 @@
if(!WriteFdExactly(s, &msg.dent, sizeof(msg.dent)) ||
!WriteFdExactly(s, de->d_name, len)) {
- closedir(d);
return -1;
}
}
}
- closedir(d);
-
done:
msg.dent.id = ID_DONE;
msg.dent.mode = 0;
diff --git a/adb/file_sync_service.h b/adb/file_sync_service.h
index 344eb98..1d3e3bd 100644
--- a/adb/file_sync_service.h
+++ b/adb/file_sync_service.h
@@ -68,9 +68,9 @@
void file_sync_service(int fd, void *cookie);
int do_sync_ls(const char *path);
-int do_sync_push(const char *lpath, const char *rpath, int show_progress);
+int do_sync_push(const char *lpath, const char *rpath, bool show_progress);
int do_sync_sync(const std::string& lpath, const std::string& rpath, bool list_only);
-int do_sync_pull(const char *rpath, const char *lpath, int show_progress, int pullTime);
+int do_sync_pull(const char *rpath, const char *lpath, bool show_progress, int pullTime);
#define SYNC_DATA_MAX (64*1024)
diff --git a/adb/sysdeps.h b/adb/sysdeps.h
index 3a3ffda..c033999 100644
--- a/adb/sysdeps.h
+++ b/adb/sysdeps.h
@@ -58,6 +58,7 @@
#include "fdevent.h"
+#define OS_PATH_SEPARATORS "\\/"
#define OS_PATH_SEPARATOR '\\'
#define OS_PATH_SEPARATOR_STR "\\"
#define ENV_PATH_SEPARATOR_STR ";"
@@ -122,9 +123,8 @@
#undef unlink
#define unlink ___xxx_unlink
-static __inline__ int adb_mkdir(const std::string& path, int mode)
-{
- return _mkdir(path);
+static __inline__ int adb_mkdir(const std::string& path, int mode) {
+ return _mkdir(path.c_str());
}
#undef mkdir
#define mkdir ___xxx_mkdir
@@ -236,32 +236,7 @@
extern int adb_socketpair( int sv[2] );
-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 == std::string::npos )
- p = p2;
- else if ( p2 != std::string::npos && p2 > p )
- p = p2;
-
- return p;
-}
-
-static __inline__ size_t adb_dirstop(const std::string& path) {
- size_t p = path.rfind('/');
- size_t p2 = path.rfind('\\');
-
- if ( p == std::string::npos )
- p = p2;
- else if ( p2 != std::string::npos && p2 > p )
- p = p2;
-
- return p;
-}
-
-static __inline__ int adb_is_absolute_host_path( const char* path )
-{
+static __inline__ int adb_is_absolute_host_path(const char* path) {
return isalpha(path[0]) && path[1] == ':' && path[2] == '\\';
}
@@ -286,6 +261,7 @@
#include <string>
+#define OS_PATH_SEPARATORS "/"
#define OS_PATH_SEPARATOR '/'
#define OS_PATH_SEPARATOR_STR "/"
#define ENV_PATH_SEPARATOR_STR ":"
@@ -524,14 +500,6 @@
{
}
-static __inline__ size_t adb_dirstart(const std::string& path, size_t pos = 0) {
- return path.find('/', pos);
-}
-
-static __inline__ size_t adb_dirstop(const std::string& path) {
- return path.rfind('/');
-}
-
static __inline__ int adb_is_absolute_host_path(const char* path) {
return path[0] == '/';
}
