Fix "adb remount" for devices without an oem partition.

On a device without an oem partition, we now have an /oem directory
anyway. This causes find_mount to fail, and that was returning nullptr
from a std::string-returning function. Boom!

Also clean up the bits of code I had to trace through between "adb remount"
on the host to the crash on the device as I debugged this.

The only other meaningful change is the error checking in
adb_connect_command --- adb_connect can also return -2.

Bug: http://b/20916855
Change-Id: I4c3b7858e13f3a3a8bbc7d30b3c0ee470bead587
(cherry picked from commit 5677c23e8d0c085be8d8429a5d125147d11e9bb2)

2 files changed, 37 insertions, 53 deletions

diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index af05dc0..cef5f39 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -263,23 +263,16 @@ static void stdin_raw_restore(int fd) {
 }
 #endif
 
-static void read_and_dump(int fd)
-{
-    char buf[4096];
-    int len;
-
-    while(fd >= 0) {
+static void read_and_dump(int fd) {
+    while (fd >= 0) {
         D("read_and_dump(): pre adb_read(fd=%d)\n", fd);
-        len = adb_read(fd, buf, 4096);
+        char buf[BUFSIZ];
+        int len = adb_read(fd, buf, sizeof(buf));
         D("read_and_dump(): post adb_read(fd=%d): len=%d\n", fd, len);
-        if(len == 0) {
+        if (len <= 0) {
             break;
         }
 
-        if(len < 0) {
-            if(errno == EINTR) continue;
-            break;
-        }
         fwrite(buf, 1, len, stdout);
         fflush(stdout);
     }
@@ -928,13 +921,13 @@ static void parse_push_pull_args(const char **arg, int narg, char const **path1,
 static int adb_connect_command(const std::string& command) {
     std::string error;
     int fd = adb_connect(command, &error);
-    if (fd != -1) {
-        read_and_dump(fd);
-        adb_close(fd);
-        return 0;
+    if (fd < 0) {
+        fprintf(stderr, "error: %s\n", error.c_str());
+        return 1;
     }
-    fprintf(stderr, "Error: %s\n", error.c_str());
-    return 1;
+    read_and_dump(fd);
+    adb_close(fd);
+    return 0;
 }
 
 static int adb_query_command(const std::string& command) {
@@ -1253,13 +1246,13 @@ int adb_commandline(int argc, const char **argv) {
              !strcmp(argv[0], "unroot") ||
              !strcmp(argv[0], "disable-verity") ||
              !strcmp(argv[0], "enable-verity")) {
-        char command[100];
+        std::string command;
         if (!strcmp(argv[0], "reboot-bootloader")) {
-            snprintf(command, sizeof(command), "reboot:bootloader");
+            command = "reboot:bootloader";
         } else if (argc > 1) {
-            snprintf(command, sizeof(command), "%s:%s", argv[0], argv[1]);
+            command = android::base::StringPrintf("%s:%s", argv[0], argv[1]);
         } else {
-            snprintf(command, sizeof(command), "%s:", argv[0]);
+            command = android::base::StringPrintf("%s:", argv[0]);
         }
         return adb_connect_command(command);
     }
diff --git a/adb/remount_service.cpp b/adb/remount_service.cpp
index 87702b0..f645a78 100644
--- a/adb/remount_service.cpp
+++ b/adb/remount_service.cpp
@@ -38,23 +38,20 @@ static int system_ro = 1;
 static int vendor_ro = 1;
 static int oem_ro = 1;
 
-/* Returns the device used to mount a directory in /proc/mounts */
-static std::string find_mount(const char *dir) {
-    FILE* fp;
-    struct mntent* mentry;
-    char* device = NULL;
-
-    if ((fp = setmntent("/proc/mounts", "r")) == NULL) {
-        return NULL;
+// Returns the device used to mount a directory in /proc/mounts.
+static std::string find_mount(const char* dir) {
+    std::unique_ptr<FILE, int(*)(FILE*)> fp(setmntent("/proc/mounts", "r"), endmntent);
+    if (!fp) {
+        return "";
     }
-    while ((mentry = getmntent(fp)) != NULL) {
-        if (strcmp(dir, mentry->mnt_dir) == 0) {
-            device = mentry->mnt_fsname;
-            break;
+
+    mntent* e;
+    while ((e = getmntent(fp.get())) != nullptr) {
+        if (strcmp(dir, e->mnt_dir) == 0) {
+            return e->mnt_fsname;
         }
     }
-    endmntent(fp);
-    return device;
+    return "";
 }
 
 int make_block_device_writable(const std::string& dev) {
@@ -75,7 +72,7 @@ int make_block_device_writable(const std::string& dev) {
 
 // Init mounts /system as read only, remount to enable writes.
 static int remount(const char* dir, int* dir_ro) {
-    std::string dev(find_mount(dir));
+    std::string dev = find_mount(dir);
     if (dev.empty() || make_block_device_writable(dev)) {
         return -1;
     }
@@ -86,35 +83,29 @@ static int remount(const char* dir, int* dir_ro) {
 }
 
 static bool remount_partition(int fd, const char* partition, int* ro) {
-  if (!directory_exists(partition)) {
+    if (!directory_exists(partition)) {
+        return true;
+    }
+    if (remount(partition, ro)) {
+        WriteFdFmt(fd, "remount of %s failed: %s\n", partition, strerror(errno));
+        return false;
+    }
     return true;
-  }
-  if (remount(partition, ro)) {
-    WriteFdFmt(fd, "remount of %s failed: %s\n", partition, strerror(errno));
-    return false;
-  }
-  return true;
 }
 
 void remount_service(int fd, void* cookie) {
-    char prop_buf[PROPERTY_VALUE_MAX];
-
     if (getuid() != 0) {
         WriteFdExactly(fd, "Not running as root. Try \"adb root\" first.\n");
         adb_close(fd);
         return;
     }
 
-    bool system_verified = false, vendor_verified = false;
+    char prop_buf[PROPERTY_VALUE_MAX];
     property_get("partition.system.verified", prop_buf, "");
-    if (strlen(prop_buf) > 0) {
-        system_verified = true;
-    }
+    bool system_verified = (strlen(prop_buf) > 0);
 
     property_get("partition.vendor.verified", prop_buf, "");
-    if (strlen(prop_buf) > 0) {
-        vendor_verified = true;
-    }
+    bool vendor_verified = (strlen(prop_buf) > 0);
 
     if (system_verified || vendor_verified) {
         // Allow remount but warn of likely bad effects