am 186f1dee: Merge "adb sync cleanup."

* commit '186f1dee2afc15c8941a1f78e1aa97e2ec9970ee':
  adb sync cleanup.

adb/SERVICES.TXT

@@ -237,7 +237,7 @@ track-jdwp
     Note that there is no single-shot service to retrieve the list only once.
 
 sync:
-    This starts the file synchronisation service, used to implement "adb push"
+    This starts the file synchronization service, used to implement "adb push"
     and "adb pull". Since this service is pretty complex, it will be detailed
     in a companion document named SYNC.TXT
 

adb/SYNC.TXT

@@ -25,12 +25,9 @@ follows as described in SERVICES.TXT.
 
 The following sync requests are accepted:
 LIST - List the files in a folder
+RECV - Retrieve a file from device
 SEND - Send a file to device
-RECV - Retreive a file from device
-
-Not yet documented:
 STAT - Stat a file
-ULNK - Unlink (remove) a file. (Not currently supported)
 
 For all of the sync request above the must be followed by length number of
 bytes containing an utf-8 string with a remote filename.
@@ -40,7 +37,7 @@ Lists files in the directory specified by the remote filename. The server will
 respond with zero or more directory entries or "dents".
 
 The directory entries will be returned in the following form
-1. A four-byte sync response id beeing "DENT"
+1. A four-byte sync response id "DENT"
 2. A four-byte integer representing file mode.
 3. A four-byte integer representing file size.
 4. A four-byte integer representing last modified time.
@@ -60,13 +57,13 @@ the transfer fails. Some file types will not be deleted, which allows
   adb push disk_image /some_block_device
 to work.
 
-After this the actual file is sent in chunks. Each chucks has the following
+After this the actual file is sent in chunks. Each chunk has the following
 format.
 A sync request with id "DATA" and length equal to the chunk size. After
 follows chunk size number of bytes. This is repeated until the file is
-transfered. Each chunk must not be larger than 64k.
+transferred. Each chunk must not be larger than 64k.
 
-When the file is tranfered a sync request "DONE" is sent, where length is set
+When the file is transferred a sync request "DONE" is sent, where length is set
 to the last modified time for the file. The server responds to this last
 request (but not to chuck requests) with an "OKAY" sync response (length can
 be ignored).
@@ -77,8 +74,8 @@ Retrieves a file from device to a local file. The remote path is the path to
 the file that will be returned. Just as for the SEND sync request the file
 received is split up into chunks. The sync response id is "DATA" and length is
 the chuck size. After follows chunk size number of bytes. This is repeated
-until the file is transfered. Each chuck will not be larger than 64k.
+until the file is transferred. Each chuck will not be larger than 64k.
 
-When the file is transfered a sync resopnse "DONE" is retrieved where the
+When the file is transferred a sync response "DONE" is retrieved where the
 length can be ignored.
 

adb/adb_client.cpp

@@ -44,28 +44,6 @@ static const char* __adb_serial = NULL;
 static int __adb_server_port = DEFAULT_ADB_PORT;
 static const char* __adb_server_name = NULL;
 
-static std::string perror_str(const char* msg) {
-    return android::base::StringPrintf("%s: %s", msg, strerror(errno));
-}
-
-static bool ReadProtocolString(int fd, std::string* s, std::string* error) {
-    char buf[5];
-    if (!ReadFdExactly(fd, buf, 4)) {
-        *error = perror_str("protocol fault (couldn't read status length)");
-        return false;
-    }
-    buf[4] = 0;
-
-    unsigned long len = strtoul(buf, 0, 16);
-    s->resize(len, '\0');
-    if (!ReadFdExactly(fd, &(*s)[0], len)) {
-        *error = perror_str("protocol fault (couldn't read status message)");
-        return false;
-    }
-
-    return true;
-}
-
 void adb_set_transport(TransportType type, const char* serial)
 {
     __adb_transport = type;
@@ -175,7 +153,7 @@ int _adb_connect(const std::string& service, std::string* error) {
         return -1;
     }
 
-    if(!SendProtocolString(fd, service)) {
+    if (!SendProtocolString(fd, service)) {
         *error = perror_str("write failure during connection");
         adb_close(fd);
         return -1;

adb/adb_io.cpp

@@ -32,7 +32,27 @@ bool SendProtocolString(int fd, const std::string& s) {
         length = 0xffff;
     }
 
-    return WriteFdFmt(fd, "%04x", length) && WriteFdExactly(fd, s);
+    // The cost of sending two strings outweighs the cost of formatting.
+    // "adb sync" performance is affected by this.
+    return WriteFdFmt(fd, "%04x%.*s", length, length, s.c_str());
+}
+
+bool ReadProtocolString(int fd, std::string* s, std::string* error) {
+    char buf[5];
+    if (!ReadFdExactly(fd, buf, 4)) {
+        *error = perror_str("protocol fault (couldn't read status length)");
+        return false;
+    }
+    buf[4] = 0;
+
+    unsigned long len = strtoul(buf, 0, 16);
+    s->resize(len, '\0');
+    if (!ReadFdExactly(fd, &(*s)[0], len)) {
+        *error = perror_str("protocol fault (couldn't read status message)");
+        return false;
+    }
+
+    return true;
 }
 
 bool SendOkay(int fd) {

adb/adb_io.h

@@ -30,23 +30,22 @@ bool SendFail(int fd, const std::string& reason);
 // Writes a protocol-format string; a four hex digit length followed by the string data.
 bool SendProtocolString(int fd, const std::string& s);
 
-/*
+// Reads a protocol-format string; a four hex digit length followed by the string data.
- * Reads exactly len bytes from fd into buf.
+bool ReadProtocolString(int fd, std::string* s, std::string* error);
- *
- * Returns false if there is an error or if EOF was reached before len bytes
- * were read. If EOF was found, errno will be set to 0.
- *
- * If this function fails, the contents of buf are undefined.
- */
-bool ReadFdExactly(int fd, void *buf, size_t len);
 
-/*
+// Reads exactly len bytes from fd into buf.
- * Writes exactly len bytes from buf to fd.
+//
- *
+// Returns false if there is an error or if EOF was reached before len bytes
- * Returns false if there is an error or if the fd was closed before the write
+// were read. If EOF was found, errno will be set to 0.
- * completed. If the other end of the fd (such as in a socket, pipe, or fifo),
+//
- * is closed, errno will be set to 0.
+// If this function fails, the contents of buf are undefined.
- */
+bool ReadFdExactly(int fd, void* buf, size_t len);
+
+// Writes exactly len bytes from buf to fd.
+//
+// Returns false if there is an error or if the fd was closed before the write
+// completed. If the other end of the fd (such as in a socket, pipe, or fifo),
+// is closed, errno will be set to 0.
 bool WriteFdExactly(int fd, const void* buf, size_t len);
 
 // Same as above, but for strings.

adb/adb_utils.cpp

@@ -225,3 +225,7 @@ bool parse_host_and_port(const std::string& address,
                << " (" << *canonical_address << ")";
     return true;
 }
+
+std::string perror_str(const char* msg) {
+    return android::base::StringPrintf("%s: %s", msg, strerror(errno));
+}

adb/adb_utils.h

@@ -44,4 +44,6 @@ bool parse_host_and_port(const std::string& address,
                          std::string* host, int* port,
                          std::string* error);
 
+std::string perror_str(const char* msg);
+
 #endif

adb/commandline.cpp

@@ -381,7 +381,7 @@ static void *stdin_read_thread(void *x)
     fdi = fds[1];
     free(fds);
 
-    for(;;) {
+    while (true) {
         /* fdi is really the client's stdin, so use read, not adb_read here */
         D("stdin_read_thread(): pre unix_read(fdi=%d,...)\n", fdi);
         r = unix_read(fdi, buf, 1024);
@@ -889,14 +889,14 @@ static std::string find_product_out_path(const std::string& hint) {
 }
 
 static void parse_push_pull_args(const char **arg, int narg, char const **path1,
-                                 char const **path2, int *show_progress,
+                                 char const **path2, bool* show_progress,
                                  int *copy_attrs) {
-    *show_progress = 0;
+    *show_progress = false;
     *copy_attrs = 0;
 
     while (narg > 0) {
         if (!strcmp(*arg, "-p")) {
-            *show_progress = 1;
+            *show_progress = true;
         } else if (!strcmp(*arg, "-a")) {
             *copy_attrs = 1;
         } else {
@@ -1331,33 +1331,25 @@ int adb_commandline(int argc, const char **argv) {
     /* do_sync_*() commands */
     else if (!strcmp(argv[0], "ls")) {
         if (argc != 2) return usage();
-        return do_sync_ls(argv[1]);
+        return do_sync_ls(argv[1]) ? 0 : 1;
     }
     else if (!strcmp(argv[0], "push")) {
-        int show_progress = 0;
+        bool show_progress = false;
-        int copy_attrs = 0; // unused
+        int copy_attrs = 0;
         const char* lpath = NULL, *rpath = NULL;
 
         parse_push_pull_args(&argv[1], argc - 1, &lpath, &rpath, &show_progress, &copy_attrs);
-
+        if (!lpath || !rpath || copy_attrs != 0) return usage();
-        if ((lpath != NULL) && (rpath != NULL)) {
+        return do_sync_push(lpath, rpath, show_progress) ? 0 : 1;
-            return do_sync_push(lpath, rpath, show_progress);
-        }
-
-        return usage();
     }
     else if (!strcmp(argv[0], "pull")) {
-        int show_progress = 0;
+        bool show_progress = false;
         int copy_attrs = 0;
         const char* rpath = NULL, *lpath = ".";
 
         parse_push_pull_args(&argv[1], argc - 1, &rpath, &lpath, &show_progress, &copy_attrs);
-
+        if (!rpath) return usage();
-        if (rpath != NULL) {
+        return do_sync_pull(rpath, lpath, show_progress, copy_attrs) ? 0 : 1;
-            return do_sync_pull(rpath, lpath, show_progress, copy_attrs);
-        }
-
-        return usage();
     }
     else if (!strcmp(argv[0], "install")) {
         if (argc < 2) return usage();
@@ -1401,20 +1393,20 @@ int adb_commandline(int argc, const char **argv) {
         std::string vendor_src_path = product_file("vendor");
         std::string oem_src_path = product_file("oem");
 
-        int rc = 0;
+        bool okay = true;
-        if (rc == 0 && (src.empty() || src == "system")) {
+        if (okay && (src.empty() || src == "system")) {
-            rc = do_sync_sync(system_src_path, "/system", list_only);
+            okay = do_sync_sync(system_src_path, "/system", list_only);
         }
-        if (rc == 0 && (src.empty() || src == "vendor") && directory_exists(vendor_src_path)) {
+        if (okay && (src.empty() || src == "vendor") && directory_exists(vendor_src_path)) {
-            rc = do_sync_sync(vendor_src_path, "/vendor", list_only);
+            okay = do_sync_sync(vendor_src_path, "/vendor", list_only);
         }
-        if (rc == 0 && (src.empty() || src == "oem") && directory_exists(oem_src_path)) {
+        if (okay && (src.empty() || src == "oem") && directory_exists(oem_src_path)) {
-            rc = do_sync_sync(oem_src_path, "/oem", list_only);
+            okay = do_sync_sync(oem_src_path, "/oem", list_only);
         }
-        if (rc == 0 && (src.empty() || src == "data")) {
+        if (okay && (src.empty() || src == "data")) {
-            rc = do_sync_sync(data_src_path, "/data", list_only);
+            okay = do_sync_sync(data_src_path, "/data", list_only);
         }
-        return rc;
+        return okay ? 0 : 1;
     }
     /* passthrough commands */
     else if (!strcmp(argv[0],"get-state") ||
@@ -1535,20 +1527,16 @@ static int install_app(TransportType transport, const char* serial, int argc, co
         return -1;
     }
 
+    int result = -1;
     const char* apk_file = argv[last_apk];
     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 (!do_sync_push(apk_file, apk_dest.c_str(), false)) goto cleanup_apk;
-    if (err) {
+    argv[last_apk] = apk_dest.c_str(); /* destination name, not source location */
-        goto cleanup_apk;
+    result = pm_command(transport, serial, argc, argv);
-    } else {
-        argv[last_apk] = apk_dest.c_str(); /* destination name, not source location */
-    }
-
-    err = pm_command(transport, serial, argc, argv);
 
 cleanup_apk:
     delete_file(transport, serial, apk_dest);
-    return err;
+    return result;
 }
 
 static int install_multiple_app(TransportType transport, const char* serial, int argc,

adb/fdevent.cpp

@@ -688,7 +688,7 @@ void fdevent_loop()
     fdevent_subproc_setup();
 #endif // !ADB_HOST
 
-    for(;;) {
+    while (true) {
         D("--- ---- waiting for events\n");
 
         fdevent_process();

adb/file_sync_service.cpp

@@ -34,6 +34,7 @@
 #include "adb_io.h"
 #include "private/android_filesystem_config.h"
 
+#include <base/stringprintf.h>
 #include <base/strings.h>
 
 static bool should_use_fs_config(const std::string& path) {
@@ -76,38 +77,23 @@ static bool secure_mkdirs(const std::string& path) {
     return true;
 }
 
-static int do_stat(int s, const char *path)
+static bool do_stat(int s, const char* path) {
-{
     syncmsg msg;
-    struct stat st;
-
     msg.stat.id = ID_STAT;
 
-    if(lstat(path, &st)) {
+    struct stat st;
-        msg.stat.mode = 0;
+    memset(&st, 0, sizeof(st));
-        msg.stat.size = 0;
+    // TODO: add a way to report that the stat failed!
-        msg.stat.time = 0;
+    lstat(path, &st);
-    } else {
+    msg.stat.mode = htoll(st.st_mode);
-        msg.stat.mode = htoll(st.st_mode);
+    msg.stat.size = htoll(st.st_size);
-        msg.stat.size = htoll(st.st_size);
+    msg.stat.time = htoll(st.st_mtime);
-        msg.stat.time = htoll(st.st_mtime);
-    }
 
-    return WriteFdExactly(s, &msg.stat, sizeof(msg.stat)) ? 0 : -1;
+    return WriteFdExactly(s, &msg.stat, sizeof(msg.stat));
 }
 
-static int do_list(int s, const char *path)
+static bool do_list(int s, const char* path) {
-{
+    dirent* de;
-    struct dirent *de;
-    struct stat st;
-
-    char tmp[1024 + 256 + 1];
-    char *fname;
-
-    size_t len = strlen(path);
-    memcpy(tmp, path, len);
-    tmp[len] = '/';
-    fname = tmp + len + 1;
 
     syncmsg msg;
     msg.dent.id = ID_DENT;
@@ -116,22 +102,19 @@ static int do_list(int s, const char *path)
     if (!d) goto done;
 
     while ((de = readdir(d.get()))) {
-        int len = strlen(de->d_name);
+        std::string filename(android::base::StringPrintf("%s/%s", path, de->d_name));
 
-            /* not supposed to be possible, but
+        struct stat st;
-               if it does happen, let's not buffer overrun */
+        if (lstat(filename.c_str(), &st) == 0) {
-        if(len > 256) continue;
+            size_t d_name_length = strlen(de->d_name);
-
-        strcpy(fname, de->d_name);
-        if(lstat(tmp, &st) == 0) {
             msg.dent.mode = htoll(st.st_mode);
             msg.dent.size = htoll(st.st_size);
             msg.dent.time = htoll(st.st_mtime);
-            msg.dent.namelen = htoll(len);
+            msg.dent.namelen = htoll(d_name_length);
 
-            if(!WriteFdExactly(s, &msg.dent, sizeof(msg.dent)) ||
+            if (!WriteFdExactly(s, &msg.dent, sizeof(msg.dent)) ||
-               !WriteFdExactly(s, de->d_name, len)) {
+                    !WriteFdExactly(s, de->d_name, d_name_length)) {
-                return -1;
+                return false;
             }
         }
     }
@@ -142,43 +125,33 @@ done:
     msg.dent.size = 0;
     msg.dent.time = 0;
     msg.dent.namelen = 0;
-    return WriteFdExactly(s, &msg.dent, sizeof(msg.dent)) ? 0 : -1;
+    return WriteFdExactly(s, &msg.dent, sizeof(msg.dent));
 }
 
-static int fail_message(int s, const char *reason)
+static bool fail_message(int s, const std::string& reason) {
-{
+    D("sync: failure: %s\n", reason.c_str());
+
     syncmsg msg;
-    int len = strlen(reason);
-
-    D("sync: failure: %s\n", reason);
-
     msg.data.id = ID_FAIL;
-    msg.data.size = htoll(len);
+    msg.data.size = htoll(reason.size());
-    if(!WriteFdExactly(s, &msg.data, sizeof(msg.data)) ||
+    return WriteFdExactly(s, &msg.data, sizeof(msg.data)) && WriteFdExactly(s, reason);
-       !WriteFdExactly(s, reason, len)) {
-        return -1;
-    } else {
-        return 0;
-    }
 }
 
-static int fail_errno(int s)
+// TODO: callers of this have already failed, and should probably ignore its
-{
+// return value (http://b/23437039).
+static bool fail_errno(int s) {
     return fail_message(s, strerror(errno));
 }
 
-static int handle_send_file(int s, char *path, uid_t uid,
+static bool handle_send_file(int s, char *path, uid_t uid,
-        gid_t gid, mode_t mode, char *buffer, bool do_unlink)
+                             gid_t gid, mode_t mode, std::vector<char>& buffer, bool do_unlink) {
-{
     syncmsg msg;
     unsigned int timestamp = 0;
-    int fd;
 
-    fd = adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, mode);
+    int fd = adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, mode);
     if(fd < 0 && errno == ENOENT) {
         if (!secure_mkdirs(path)) {
-            if(fail_errno(s))
+            if (fail_errno(s)) return false;
-                return -1;
             fd = -1;
         } else {
             fd = adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, mode);
@@ -188,8 +161,7 @@ static int handle_send_file(int s, char *path, uid_t uid,
         fd = adb_open_mode(path, O_WRONLY | O_CLOEXEC, mode);
     }
     if(fd < 0) {
-        if(fail_errno(s))
+        if (fail_errno(s)) return false;
-            return -1;
         fd = -1;
     } else {
         if(fchown(fd, uid, gid) != 0) {
@@ -205,7 +177,7 @@ static int handle_send_file(int s, char *path, uid_t uid,
         fchmod(fd, mode);
     }
 
-    for(;;) {
+    while (true) {
         unsigned int len;
 
         if(!ReadFdExactly(s, &msg.data, sizeof(msg.data)))
@@ -220,22 +192,21 @@ static int handle_send_file(int s, char *path, uid_t uid,
             goto fail;
         }
         len = ltohl(msg.data.size);
-        if(len > SYNC_DATA_MAX) {
+        if (len > buffer.size()) { // TODO: resize buffer?
             fail_message(s, "oversize data message");
             goto fail;
         }
-        if(!ReadFdExactly(s, buffer, len))
+        if (!ReadFdExactly(s, &buffer[0], len)) goto fail;
-            goto fail;
 
-        if(fd < 0)
+        if (fd < 0) continue;
-            continue;
+
-        if(!WriteFdExactly(fd, buffer, len)) {
+        if (!WriteFdExactly(fd, &buffer[0], len)) {
             int saved_errno = errno;
             adb_close(fd);
             if (do_unlink) adb_unlink(path);
             fd = -1;
             errno = saved_errno;
-            if(fail_errno(s)) return -1;
+            if (fail_errno(s)) return false;
         }
     }
 
@@ -249,75 +220,67 @@ static int handle_send_file(int s, char *path, uid_t uid,
 
         msg.status.id = ID_OKAY;
         msg.status.msglen = 0;
-        if(!WriteFdExactly(s, &msg.status, sizeof(msg.status)))
+        if (!WriteFdExactly(s, &msg.status, sizeof(msg.status))) return false;
-            return -1;
     }
-    return 0;
+    return true;
 
 fail:
-    if(fd >= 0)
+    if (fd >= 0) adb_close(fd);
-        adb_close(fd);
     if (do_unlink) adb_unlink(path);
-    return -1;
+    return false;
 }
 
 #if defined(_WIN32)
-extern int handle_send_link(int s, char *path, char *buffer) __attribute__((error("no symlinks on Windows")));
+extern bool handle_send_link(int s, char *path, std::vector<char>& buffer) __attribute__((error("no symlinks on Windows")));
 #else
-static int handle_send_link(int s, char *path, char *buffer)
+static bool handle_send_link(int s, char *path, std::vector<char>& buffer) {
-{
     syncmsg msg;
     unsigned int len;
     int ret;
 
-    if(!ReadFdExactly(s, &msg.data, sizeof(msg.data)))
+    if (!ReadFdExactly(s, &msg.data, sizeof(msg.data))) return false;
-        return -1;
 
-    if(msg.data.id != ID_DATA) {
+    if (msg.data.id != ID_DATA) {
         fail_message(s, "invalid data message: expected ID_DATA");
-        return -1;
+        return false;
     }
 
     len = ltohl(msg.data.size);
-    if(len > SYNC_DATA_MAX) {
+    if (len > buffer.size()) { // TODO: resize buffer?
         fail_message(s, "oversize data message");
-        return -1;
+        return false;
     }
-    if(!ReadFdExactly(s, buffer, len))
+    if (!ReadFdExactly(s, &buffer[0], len)) return false;
-        return -1;
 
-    ret = symlink(buffer, path);
+    ret = symlink(&buffer[0], path);
-    if(ret && errno == ENOENT) {
+    if (ret && errno == ENOENT) {
         if (!secure_mkdirs(path)) {
             fail_errno(s);
-            return -1;
+            return false;
         }
-        ret = symlink(buffer, path);
+        ret = symlink(&buffer[0], path);
     }
-    if(ret) {
+    if (ret) {
         fail_errno(s);
-        return -1;
+        return false;
     }
 
-    if(!ReadFdExactly(s, &msg.data, sizeof(msg.data)))
+    if (!ReadFdExactly(s, &msg.data, sizeof(msg.data))) return false;
-        return -1;
 
-    if(msg.data.id == ID_DONE) {
+    if (msg.data.id == ID_DONE) {
         msg.status.id = ID_OKAY;
         msg.status.msglen = 0;
-        if(!WriteFdExactly(s, &msg.status, sizeof(msg.status)))
+        if (!WriteFdExactly(s, &msg.status, sizeof(msg.status))) return false;
-            return -1;
     } else {
         fail_message(s, "invalid data message: expected ID_DONE");
-        return -1;
+        return false;
     }
 
-    return 0;
+    return true;
 }
 #endif
 
-static int do_send(int s, char *path, char *buffer)
+static bool do_send(int s, char* path, std::vector<char>& buffer) {
-{
     unsigned int mode;
     bool is_link = false;
     bool do_unlink;
@@ -365,32 +328,28 @@ static int do_send(int s, char *path, char *buffer)
     return handle_send_file(s, path, uid, gid, mode, buffer, do_unlink);
 }
 
-static int do_recv(int s, const char *path, char *buffer)
+static bool do_recv(int s, const char* path, std::vector<char>& buffer) {
-{
+    int fd = adb_open(path, O_RDONLY | O_CLOEXEC);
-    syncmsg msg;
+    if (fd < 0) {
-    int fd, r;
+        if (fail_errno(s)) return false;
-
+        return true;
-    fd = adb_open(path, O_RDONLY | O_CLOEXEC);
-    if(fd < 0) {
-        if(fail_errno(s)) return -1;
-        return 0;
     }
 
+    syncmsg msg;
     msg.data.id = ID_DATA;
-    for(;;) {
+    while (true) {
-        r = adb_read(fd, buffer, SYNC_DATA_MAX);
+        int r = adb_read(fd, &buffer[0], buffer.size());
-        if(r <= 0) {
+        if (r <= 0) {
-            if(r == 0) break;
+            if (r == 0) break;
-            if(errno == EINTR) continue;
+            if (errno == EINTR) continue;
-            r = fail_errno(s);
+            bool status = fail_errno(s);
             adb_close(fd);
-            return r;
+            return status;
         }
         msg.data.size = htoll(r);
-        if(!WriteFdExactly(s, &msg.data, sizeof(msg.data)) ||
+        if (!WriteFdExactly(s, &msg.data, sizeof(msg.data)) || !WriteFdExactly(s, &buffer[0], r)) {
-           !WriteFdExactly(s, buffer, r)) {
             adb_close(fd);
-            return -1;
+            return false;
         }
     }
 
@@ -398,66 +357,62 @@ static int do_recv(int s, const char *path, char *buffer)
 
     msg.data.id = ID_DONE;
     msg.data.size = 0;
-    if(!WriteFdExactly(s, &msg.data, sizeof(msg.data))) {
+    return WriteFdExactly(s, &msg.data, sizeof(msg.data));
-        return -1;
-    }
-
-    return 0;
 }
 
-void file_sync_service(int fd, void *cookie)
+static bool handle_sync_command(int fd, std::vector<char>& buffer) {
-{
+    D("sync: waiting for request\n");
-    syncmsg msg;
+
+    SyncRequest request;
+    if (!ReadFdExactly(fd, &request, sizeof(request))) {
+        fail_message(fd, "command read failure");
+        return false;
+    }
+    size_t path_length = ltohl(request.path_length);
+    if (path_length > 1024) {
+        fail_message(fd, "path too long");
+        return false;
+    }
     char name[1025];
-    unsigned namelen;
+    if (!ReadFdExactly(fd, name, path_length)) {
+        fail_message(fd, "filename read failure");
+        return false;
+    }
+    name[path_length] = 0;
 
-    char *buffer = reinterpret_cast<char*>(malloc(SYNC_DATA_MAX));
+    const char* id = reinterpret_cast<const char*>(&request.id);
-    if(buffer == 0) goto fail;
+    D("sync: '%.4s' '%s'\n", id, name);
 
-    for(;;) {
+    switch (request.id) {
-        D("sync: waiting for command\n");
+      case ID_STAT:
-
+        if (!do_stat(fd, name)) return false;
-        if(!ReadFdExactly(fd, &msg.req, sizeof(msg.req))) {
+        break;
-            fail_message(fd, "command read failure");
+      case ID_LIST:
-            break;
+        if (!do_list(fd, name)) return false;
-        }
+        break;
-        namelen = ltohl(msg.req.namelen);
+      case ID_SEND:
-        if(namelen > 1024) {
+        if (!do_send(fd, name, buffer)) return false;
-            fail_message(fd, "invalid namelen");
+        break;
-            break;
+      case ID_RECV:
-        }
+        if (!do_recv(fd, name, buffer)) return false;
-        if(!ReadFdExactly(fd, name, namelen)) {
+        break;
-            fail_message(fd, "filename read failure");
+      case ID_QUIT:
-            break;
+        return false;
-        }
+      default:
-        name[namelen] = 0;
+        fail_message(fd, android::base::StringPrintf("unknown command '%.4s' (%08x)",
-
+                                                     id, request.id));
-        msg.req.namelen = 0;
+        return false;
-        D("sync: '%s' '%s'\n", (char*) &msg.req, name);
+    }
-
+
-        switch(msg.req.id) {
+    return true;
-        case ID_STAT:
+}
-            if(do_stat(fd, name)) goto fail;
+
-            break;
+void file_sync_service(int fd, void* cookie) {
-        case ID_LIST:
+    std::vector<char> buffer(SYNC_DATA_MAX);
-            if(do_list(fd, name)) goto fail;
+
-            break;
+    while (handle_sync_command(fd, buffer)) {
-        case ID_SEND:
-            if(do_send(fd, name, buffer)) goto fail;
-            break;
-        case ID_RECV:
-            if(do_recv(fd, name, buffer)) goto fail;
-            break;
-        case ID_QUIT:
-            goto fail;
-        default:
-            fail_message(fd, "unknown command");
-            goto fail;
-        }
     }
 
-fail:
-    if(buffer != 0) free(buffer);
     D("sync: done\n");
     adb_close(fd);
 }

adb/file_sync_service.h

@@ -26,7 +26,6 @@
 
 #define ID_STAT MKID('S','T','A','T')
 #define ID_LIST MKID('L','I','S','T')
-#define ID_ULNK MKID('U','L','N','K')
 #define ID_SEND MKID('S','E','N','D')
 #define ID_RECV MKID('R','E','C','V')
 #define ID_DENT MKID('D','E','N','T')
@@ -36,41 +35,41 @@
 #define ID_FAIL MKID('F','A','I','L')
 #define ID_QUIT MKID('Q','U','I','T')
 
+struct SyncRequest {
+    uint32_t id;  // ID_STAT, et cetera.
+    uint32_t path_length;  // <= 1024
+    // Followed by 'path_length' bytes of path (not NUL-terminated).
+} __attribute__((packed)) ;
+
 union syncmsg {
-    unsigned id;
+    struct __attribute__((packed)) {
-    struct {
-        unsigned id;
-        unsigned namelen;
-    } req;
-    struct {
         unsigned id;
         unsigned mode;
         unsigned size;
         unsigned time;
     } stat;
-    struct {
+    struct __attribute__((packed)) {
         unsigned id;
         unsigned mode;
         unsigned size;
         unsigned time;
         unsigned namelen;
     } dent;
-    struct {
+    struct __attribute__((packed)) {
         unsigned id;
         unsigned size;
     } data;
-    struct {
+    struct __attribute__((packed)) {
         unsigned id;
         unsigned msglen;
     } status;
-} ;
+};
 
-
+void file_sync_service(int fd, void* cookie);
-void file_sync_service(int fd, void *cookie);
+bool do_sync_ls(const char* path);
-int do_sync_ls(const char *path);
+bool do_sync_push(const char* lpath, const char* rpath, bool show_progress);
-int do_sync_push(const char *lpath, const char *rpath, bool show_progress);
+bool do_sync_sync(const std::string& lpath, const std::string& rpath, bool list_only);
-int do_sync_sync(const std::string& lpath, const std::string& rpath, bool list_only);
+bool do_sync_pull(const char* rpath, const char* lpath, bool show_progress, int copy_attrs);
-int do_sync_pull(const char *rpath, const char *lpath, bool show_progress, int pullTime);
 
 #define SYNC_DATA_MAX (64*1024)
 

adb/services.cpp

@@ -438,8 +438,7 @@ static int create_subproc_thread(const char *name, SubprocessType type) {
 }
 #endif
 
-int service_to_fd(const char *name)
+int service_to_fd(const char* name) {
-{
     int ret = -1;
 
     if(!strncmp(name, "tcp:", 4)) {
@@ -503,13 +502,11 @@ int service_to_fd(const char *name)
     } else if(!strncmp(name, "unroot:", 7)) {
         ret = create_service_thread(restart_unroot_service, NULL);
     } else if(!strncmp(name, "backup:", 7)) {
-        ret = create_subproc_thread(
+        ret = create_subproc_thread(android::base::StringPrintf("/system/bin/bu backup %s",
-                android::base::StringPrintf("/system/bin/bu backup %s",
+                                                                (name + 7)).c_str(),
-                                            (name + 7)).c_str(),
-                SubprocessType::kRaw);
-    } else if(!strncmp(name, "restore:", 8)) {
-        ret = create_subproc_thread("/system/bin/bu restore",
                                     SubprocessType::kRaw);
+    } else if(!strncmp(name, "restore:", 8)) {
+        ret = create_subproc_thread("/system/bin/bu restore", SubprocessType::kRaw);
     } else if(!strncmp(name, "tcpip:", 6)) {
         int port;
         if (sscanf(name + 6, "%d", &port) != 1) {
@@ -667,8 +664,7 @@ static void connect_service(int fd, void* data) {
 #endif
 
 #if ADB_HOST
-asocket*  host_service_to_socket(const char*  name, const char *serial)
+asocket* host_service_to_socket(const char* name, const char* serial) {
-{
     if (!strcmp(name,"track-devices")) {
         return create_device_tracker();
     } else if (!strncmp(name, "wait-for-", strlen("wait-for-"))) {

adb/transport.cpp

@@ -865,8 +865,7 @@ void atransport::add_feature(const std::string& feature) {
 }
 
 bool atransport::CanUseFeature(const std::string& feature) const {
-    return has_feature(feature) &&
+    return has_feature(feature) && supported_features().count(feature) > 0;
-           supported_features().count(feature) > 0;
 }
 
 #if ADB_HOST