Fixed and improve RunCommand() when running as root.

Improved it by not try to run 'su root' on user builds.
Fixed the logic that creates the args.

BUG: 29319732
BUG: 26379932
Test: manual

Change-Id: I6b39abc8ee907f638905913425218c0a50d767c6
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index b5a3352..d1a96c1 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -110,6 +110,9 @@
 static int DumpFile(const std::string& title, const std::string& path) {
     return ds.DumpFile(title, path);
 }
+bool IsUserBuild() {
+    return ds.IsUserBuild();
+}
 
 /*
  * List of supported zip format versions.
@@ -124,7 +127,7 @@
 static constexpr char PROPERTY_EXTRA_OPTIONS[] = "dumpstate.options";
 static constexpr char PROPERTY_LAST_ID[] = "dumpstate.last_id";
 
-bool is_user_build() {
+bool Dumpstate::IsUserBuild() {
     return "user" == buildType;
 }
 
@@ -381,7 +384,7 @@
 }
 
 static void dump_raft() {
-    if (is_user_build()) {
+    if (IsUserBuild()) {
         return;
     }
 
@@ -1103,7 +1106,7 @@
         // root can run on user builds.
         CommandOptions::CommandOptionsBuilder options =
             CommandOptions::WithTimeout(rilDumpstateTimeout);
-        if (!is_user_build()) {
+        if (!IsUserBuild()) {
             options.AsRoot();
         }
         RunCommand("DUMP VENDOR RIL LOGS", {"vril-dump"}, options.Build());
@@ -1244,7 +1247,7 @@
         return false;
     }
 
-    if (is_user_build()) {
+    if (IsUserBuild()) {
         MYLOGD("Removing temporary file %s\n", bugreport_path.c_str())
         if (remove(bugreport_path.c_str())) {
             ALOGW("remove(%s): %s\n", bugreport_path.c_str(), strerror(errno));
@@ -1610,7 +1613,7 @@
     add_dir(RECOVERY_DIR, true);
     add_dir(RECOVERY_DATA_DIR, true);
     add_dir(LOGPERSIST_DATA_DIR, false);
-    if (!is_user_build()) {
+    if (!IsUserBuild()) {
         add_dir(PROFILE_DATA_DIR_CUR, true);
         add_dir(PROFILE_DATA_DIR_REF, true);
     }
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index bfa894e..9292cb4 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -215,6 +215,11 @@
     bool IsDryRun();
 
     /*
+     * Gets whether device is running a `user` build.
+     */
+    bool IsUserBuild();
+
+    /*
      * Forks a command, waits for it to finish, and returns its status.
      *
      * |title| description of the command printed on `stdout` (or empty to skip
@@ -235,8 +240,8 @@
      * description).
      * |dumpsys_args| `dumpsys` arguments (except `-t`).
      * |options| optional argument defining the command's behavior.
- * |dumpsysTimeout| when > 0, defines the value passed to `dumpsys -t` (otherwise it uses the
- * timeout from `options`)
+     * |dumpsysTimeout| when > 0, defines the value passed to `dumpsys -t` (otherwise it uses the
+     * timeout from `options`)
      */
     void RunDumpsys(const std::string& title, const std::vector<std::string>& dumpsysArgs,
                     const CommandOptions& options = CommandOptions::DEFAULT_DUMPSYS,
diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp
index e036dfa..476392e 100644
--- a/cmds/dumpstate/utils.cpp
+++ b/cmds/dumpstate/utils.cpp
@@ -772,28 +772,26 @@
         MYLOGE("No arguments on command '%s'\n", title.c_str());
         return -1;
     }
-    DurationReporter durationReporter(title);
 
     int size = fullCommand.size() + 1;  // null terminated
+    int startingIndex = 0;
     if (options.RootMode() == SU_ROOT) {
-        size += 2;  // "su" "root"
+        startingIndex = 2;  // "su" "root"
+        size += startingIndex;
     }
 
-    const char* args[size];
-
-    if (!title.empty()) {
-        printf("------ %s (", title.c_str());
-    }
+    std::vector<const char*> args;
+    args.resize(size);
 
     std::string commandString;
-    int i = 0;
     if (options.RootMode() == SU_ROOT) {
         args[0] = SU_PATH;
         commandString += SU_PATH;
         args[1] = "root";
         commandString += " root ";
     }
-    for (auto arg = fullCommand.begin(); arg < fullCommand.end(); arg++) {
+    int i = startingIndex;
+    for (auto arg = fullCommand.begin(); arg != fullCommand.end(); ++arg) {
         args[i++] = arg->c_str();
         commandString += arg->c_str();
         if (arg != fullCommand.end() - 1) {
@@ -804,11 +802,17 @@
     const char* path = args[0];
     const char* command = commandString.c_str();
 
+    if (options.RootMode() == SU_ROOT && ds.IsUserBuild()) {
+        printf("Skipping '%s' on user build.\n", command);
+        return 0;
+    }
+
     if (!title.empty()) {
-        printf("%s) ------\n", command);
+        printf("------ %s (%s) ------\n", title.c_str(), command);
     }
 
     fflush(stdout);
+    DurationReporter durationReporter(title);
 
     const std::string& loggingMessage = options.LoggingMessage();
     if (!loggingMessage.empty()) {
@@ -861,7 +865,7 @@
         sigact.sa_handler = SIG_IGN;
         sigaction(SIGPIPE, &sigact, NULL);
 
-        execvp(path, (char**)args);
+        execvp(path, (char**)args.data());
         // execvp's result will be handled after waitpid_with_timeout() below, but
         // if it failed, it's safer to exit dumpstate.
         MYLOGD("execvp on command '%s' failed (error: %s)\n", command, strerror(errno));