From 8c2198c8097f00ee08f515c65361455fd46eda2e Mon Sep 17 00:00:00 2001
From: Josh Gao <jmgao@google.com>
Date: Thu, 11 Jul 2019 14:15:32 -0700
Subject: [PATCH] adb: use shell for remount to forward return codes.

Bug: http://b/25842395
Test: manual
Change-Id: I719c86bdf573db14ca2a0bdf608065ad63f573c1
---
 adb/client/commandline.cpp     | 17 +++++++++++++++--
 adb/daemon/remount_service.cpp |  6 ++++--
 adb/transport.cpp              |  2 ++
 adb/transport.h                |  1 +
 fs_mgr/fs_mgr_remount.cpp      | 21 ++++++++++-----------
 5 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/adb/client/commandline.cpp b/adb/client/commandline.cpp
index 599e0e603..fe2bdfed3 100644
--- a/adb/client/commandline.cpp
+++ b/adb/client/commandline.cpp
@@ -1704,10 +1704,23 @@ int adb_commandline(int argc, const char** argv) {
             error_exit("tcpip: invalid port: %s", argv[1]);
         }
         return adb_connect_command(android::base::StringPrintf("tcpip:%d", port));
+    } else if (!strcmp(argv[0], "remount")) {
+        FeatureSet features;
+        std::string error;
+        if (!adb_get_feature_set(&features, &error)) {
+            fprintf(stderr, "error: %s\n", error.c_str());
+            return 1;
+        }
+
+        if (CanUseFeature(features, kFeatureRemountShell)) {
+            const char* arg[2] = {"shell", "remount"};
+            return adb_shell(2, arg);
+        } else {
+            return adb_connect_command("remount:");
+        }
     }
     // clang-format off
-    else if (!strcmp(argv[0], "remount") ||
-             !strcmp(argv[0], "reboot") ||
+    else if (!strcmp(argv[0], "reboot") ||
              !strcmp(argv[0], "reboot-bootloader") ||
              !strcmp(argv[0], "reboot-fastboot") ||
              !strcmp(argv[0], "usb") ||
diff --git a/adb/daemon/remount_service.cpp b/adb/daemon/remount_service.cpp
index ce494ee2b..6bd7855ea 100644
--- a/adb/daemon/remount_service.cpp
+++ b/adb/daemon/remount_service.cpp
@@ -48,6 +48,8 @@ static bool do_remount(int fd, const std::string& cmd) {
         dup2(fd, STDERR_FILENO);
 
         execl(kRemountCmd, kRemountCmd, cmd.empty() ? nullptr : cmd.c_str(), nullptr);
+        const char* msg = "failed to exec remount\n";
+        write(STDERR_FILENO, msg, strlen(msg));
         _exit(errno);
     }
 
@@ -83,6 +85,6 @@ static bool do_remount(int fd, const std::string& cmd) {
 }
 
 void remount_service(unique_fd fd, const std::string& cmd) {
-    const char* success = do_remount(fd.get(), cmd) ? "succeeded" : "failed";
-    WriteFdFmt(fd.get(), "remount %s\n", success);
+    do_remount(fd.get(), cmd);
+    // The remount command will print success or failure for us.
 }
diff --git a/adb/transport.cpp b/adb/transport.cpp
index 3d1d620d2..d9749ac35 100644
--- a/adb/transport.cpp
+++ b/adb/transport.cpp
@@ -73,6 +73,7 @@ const char* const kFeatureFixedPushMkdir = "fixed_push_mkdir";
 const char* const kFeatureAbb = "abb";
 const char* const kFeatureFixedPushSymlinkTimestamp = "fixed_push_symlink_timestamp";
 const char* const kFeatureAbbExec = "abb_exec";
+const char* const kFeatureRemountShell = "remount_shell";
 
 namespace {
 
@@ -1049,6 +1050,7 @@ const FeatureSet& supported_features() {
             kFeatureAbb,
             kFeatureFixedPushSymlinkTimestamp,
             kFeatureAbbExec,
+            kFeatureRemountShell,
             // Increment ADB_SERVER_VERSION when adding a feature that adbd needs
             // to know about. Otherwise, the client can be stuck running an old
             // version of the server even after upgrading their copy of adb.
diff --git a/adb/transport.h b/adb/transport.h
index f4490eded..245037e77 100644
--- a/adb/transport.h
+++ b/adb/transport.h
@@ -69,6 +69,7 @@ extern const char* const kFeatureFixedPushMkdir;
 extern const char* const kFeatureAbb;
 // adbd properly updates symlink timestamps on push.
 extern const char* const kFeatureFixedPushSymlinkTimestamp;
+extern const char* const kFeatureRemountShell;
 
 TransportId NextTransportId();
 
diff --git a/fs_mgr/fs_mgr_remount.cpp b/fs_mgr/fs_mgr_remount.cpp
index 149bee3da..93bba686a 100644
--- a/fs_mgr/fs_mgr_remount.cpp
+++ b/fs_mgr/fs_mgr_remount.cpp
@@ -82,13 +82,7 @@ const android::fs_mgr::FstabEntry* is_wrapped(const android::fs_mgr::Fstab& over
 
 void MyLogger(android::base::LogId id, android::base::LogSeverity severity, const char* tag,
               const char* file, unsigned int line, const char* message) {
-    static const char log_characters[] = "VD\0WEFF";
-    if (severity < sizeof(log_characters)) {
-        auto severity_char = log_characters[severity];
-        if (severity_char) fprintf(stderr, "%c ", severity_char);
-    }
     fprintf(stderr, "%s\n", message);
-
     static auto logd = android::base::LogdLogger();
     logd(id, severity, tag, file, line, message);
 }
@@ -107,11 +101,9 @@ void MyLogger(android::base::LogId id, android::base::LogSeverity severity, cons
 
 }  // namespace
 
-int main(int argc, char* argv[]) {
-    android::base::InitLogging(argv, MyLogger);
-
+static int do_remount(int argc, char* argv[]) {
     enum {
-        SUCCESS,
+        SUCCESS = 0,
         NOT_USERDEBUG,
         BADARG,
         NOT_ROOT,
@@ -165,7 +157,7 @@ int main(int argc, char* argv[]) {
 
     // Make sure we are root.
     if (::getuid() != 0) {
-        LOG(ERROR) << "must be run as root";
+        LOG(ERROR) << "Not running as root. Try \"adb root\" first.";
         return NOT_ROOT;
     }
 
@@ -390,3 +382,10 @@ int main(int argc, char* argv[]) {
 
     return retval;
 }
+
+int main(int argc, char* argv[]) {
+    android::base::InitLogging(argv, MyLogger);
+    int result = do_remount(argc, argv);
+    printf("remount %s\n", result ? "failed" : "succeeded");
+    return result;
+}