Otapreopt: Fix after shared-library work

Commit b63d91fd2737680351876406277b6c759f4db33c broke the installd
to otapreopt connection. Fix this. Also refactor a bit to make an
inadvertant break harder.

Bug: 25612095
Change-Id: I8e7b944ecbd5457e4ac4d6ffffdb0468618cf65f
diff --git a/cmds/installd/commands.cpp b/cmds/installd/commands.cpp
index c0c91da..8360a87 100644
--- a/cmds/installd/commands.cpp
+++ b/cmds/installd/commands.cpp
@@ -1385,6 +1385,28 @@
     return analyse_profiles(uid, pkgname);
 }
 
+static const char* parse_null(const char* arg) {
+    if (strcmp(arg, "!") == 0) {
+        return nullptr;
+    } else {
+        return arg;
+    }
+}
+
+int dexopt(const char* params[DEXOPT_PARAM_COUNT]) {
+    return dexopt(params[0],                    // apk_path
+                  atoi(params[1]),              // uid
+                  params[2],                    // pkgname
+                  params[3],                    // instruction_set
+                  atoi(params[4]),              // dexopt_needed
+                  params[5],                    // oat_dir
+                  atoi(params[6]),              // dexopt_flags
+                  params[7],                    // compiler_filter
+                  parse_null(params[8]),        // volume_uuid
+                  parse_null(params[9]));       // shared_libraries
+    static_assert(DEXOPT_PARAM_COUNT == 10U, "Unexpected dexopt param count");
+}
+
 int dexopt(const char* apk_path, uid_t uid, const char* pkgname, const char* instruction_set,
            int dexopt_needed, const char* oat_dir, int dexopt_flags, const char* compiler_filter,
            const char* volume_uuid ATTRIBUTE_UNUSED, const char* shared_libraries)
diff --git a/cmds/installd/commands.h b/cmds/installd/commands.h
index 7a42c5c..c0c39c5 100644
--- a/cmds/installd/commands.h
+++ b/cmds/installd/commands.h
@@ -56,9 +56,21 @@
 
 bool dump_profile(uid_t uid, const char *pkgname, const char *dex_files);
 
-int dexopt(const char *apk_path, uid_t uid, const char *pkgName, const char *instruction_set,
-           int dexopt_needed, const char* oat_dir, int dexopt_flags, const char* compiler_filter,
-           const char* volume_uuid, const char* shared_libraries);
+int dexopt(const char *apk_path,
+           uid_t uid,
+           const char *pkgName,
+           const char *instruction_set,
+           int dexopt_needed,
+           const char* oat_dir,
+           int dexopt_flags,
+           const char* compiler_filter,
+           const char* volume_uuid,
+           const char* shared_libraries);
+static_assert(DEXOPT_PARAM_COUNT == 10U, "Unexpected dexopt param size");
+
+// Helper for the above, converting arguments.
+int dexopt(const char* params[DEXOPT_PARAM_COUNT]);
+
 int mark_boot_complete(const char *instruction_set);
 int linklib(const char* uuid, const char* pkgname, const char* asecLibDir, int userId);
 int idmap(const char *target_path, const char *overlay_path, uid_t uid);
diff --git a/cmds/installd/installd.cpp b/cmds/installd/installd.cpp
index 061359e..9d2f71b 100644
--- a/cmds/installd/installd.cpp
+++ b/cmds/installd/installd.cpp
@@ -219,7 +219,8 @@
 // We use otapreopt_chroot to get into the chroot.
 static constexpr const char* kOtaPreopt = "/system/bin/otapreopt_chroot";
 
-static int do_ota_dexopt(char **arg, char reply[REPLY_MAX] ATTRIBUTE_UNUSED) {
+static int do_ota_dexopt(const char* args[DEXOPT_PARAM_COUNT],
+                         char reply[REPLY_MAX] ATTRIBUTE_UNUSED) {
     // Time to fork and run otapreopt.
 
     // Check that the tool exists.
@@ -231,12 +232,14 @@
 
     pid_t pid = fork();
     if (pid == 0) {
-        const char* argv[1 + 9 + 1];
+        const char* argv[1 + DEXOPT_PARAM_COUNT + 1];
         argv[0] = kOtaPreopt;
-        for (size_t i = 1; i <= 9; ++i) {
-            argv[i] = arg[i - 1];
+
+        for (size_t i = 0; i < DEXOPT_PARAM_COUNT; ++i) {
+            argv[i + 1] = args[i];
         }
-        argv[10] = nullptr;
+
+        argv[DEXOPT_PARAM_COUNT + 1] = nullptr;
 
         execv(argv[0], (char * const *)argv);
         PLOG(ERROR) << "execv(OTAPREOPT_CHROOT) failed";
@@ -252,22 +255,30 @@
     }
 }
 
+static int do_regular_dexopt(const char* args[DEXOPT_PARAM_COUNT],
+                             char reply[REPLY_MAX] ATTRIBUTE_UNUSED) {
+    return dexopt(args);
+}
+
+using DexoptFn = int (*)(const char* args[DEXOPT_PARAM_COUNT],
+                         char reply[REPLY_MAX]);
+
 static int do_dexopt(char **arg, char reply[REPLY_MAX])
 {
-    int dexopt_flags = atoi(arg[6]);
-    if ((dexopt_flags & DEXOPT_OTA) != 0) {
-      return do_ota_dexopt(arg, reply);
+    const char* args[DEXOPT_PARAM_COUNT];
+    for (size_t i = 0; i < DEXOPT_PARAM_COUNT; ++i) {
+        CHECK(arg[i] != nullptr);
+        args[i] = arg[i];
     }
-    return dexopt(arg[0],                      // apk_path
-                  atoi(arg[1]),                // uid
-                  arg[2],                      // pkgname
-                  arg[3],                      // instruction_set
-                  atoi(arg[4]),                // dexopt_needed
-                  arg[5],                      // oat_dir
-                  dexopt_flags,
-                  arg[7],                      // compiler_filter
-                  parse_null(arg[8]),          // volume_uuid
-                  parse_null(arg[9]));         // shared_libraries
+
+    int dexopt_flags = atoi(arg[6]);
+    DexoptFn dexopt_fn;
+    if ((dexopt_flags & DEXOPT_OTA) != 0) {
+        dexopt_fn = do_ota_dexopt;
+    } else {
+        dexopt_fn = do_regular_dexopt;
+    }
+    return dexopt_fn(args, reply);
 }
 
 static int do_merge_profiles(char **arg, char reply[REPLY_MAX])
diff --git a/cmds/installd/installd_constants.h b/cmds/installd/installd_constants.h
index 8513695..823b8ee 100644
--- a/cmds/installd/installd_constants.h
+++ b/cmds/installd/installd_constants.h
@@ -21,6 +21,8 @@
 namespace android {
 namespace installd {
 
+constexpr size_t DEXOPT_PARAM_COUNT = 10U;
+
 /* elements combined with a valid package name to form paths */
 
 constexpr const char* PRIMARY_USER_PREFIX = "data/";
diff --git a/cmds/installd/otapreopt.cpp b/cmds/installd/otapreopt.cpp
index ac511ec..3047198 100644
--- a/cmds/installd/otapreopt.cpp
+++ b/cmds/installd/otapreopt.cpp
@@ -188,12 +188,14 @@
 
     bool ReadPackage(int argc ATTRIBUTE_UNUSED, char** argv) {
         size_t index = 0;
-        while (index < ARRAY_SIZE(package_parameters_) &&
+        static_assert(DEXOPT_PARAM_COUNT == ARRAY_SIZE(package_parameters_),
+                      "Unexpected dexopt param count");
+        while (index < DEXOPT_PARAM_COUNT &&
                 argv[index + 1] != nullptr) {
             package_parameters_[index] = argv[index + 1];
             index++;
         }
-        if (index != ARRAY_SIZE(package_parameters_)) {
+        if (index != ARRAY_SIZE(package_parameters_) || argv[index + 1] != nullptr) {
             LOG(ERROR) << "Wrong number of parameters";
             return false;
         }
@@ -357,17 +359,7 @@
     }
 
     int RunPreopt() {
-        int ret = dexopt(package_parameters_[0],          // apk_path
-                atoi(package_parameters_[1]),             // uid
-                package_parameters_[2],                   // pkgname
-                package_parameters_[3],                   // instruction_set
-                atoi(package_parameters_[4]),             // dexopt_needed
-                package_parameters_[5],                   // oat_dir
-                atoi(package_parameters_[6]),             // dexopt_flags
-                package_parameters_[7],                   // compiler_filter
-                ParseNull(package_parameters_[8]),        // volume_uuid
-                ParseNull(package_parameters_[9]));       // shared_libraries
-        return ret;
+        return dexopt(package_parameters_);
     }
 
     ////////////////////////////////////
@@ -484,7 +476,7 @@
     // to compile, instead of the A properties we could get from init/get_property.
     SystemProperties system_properties_;
 
-    const char* package_parameters_[10];
+    const char* package_parameters_[DEXOPT_PARAM_COUNT];
 
     // Store environment values we need to set.
     std::vector<std::string> environ_;
diff --git a/cmds/installd/otapreopt_chroot.cpp b/cmds/installd/otapreopt_chroot.cpp
index f7f69a9..be0ff2e 100644
--- a/cmds/installd/otapreopt_chroot.cpp
+++ b/cmds/installd/otapreopt_chroot.cpp
@@ -22,6 +22,8 @@
 #include <android-base/macros.h>
 #include <android-base/stringprintf.h>
 
+#include <installd_constants.h>
+
 #ifndef LOG_TAG
 #define LOG_TAG "otapreopt"
 #endif
@@ -78,13 +80,13 @@
 
     // Now go on and run otapreopt.
 
-    const char* argv[1 + 9 + 1];
-    CHECK_EQ(argc, 10);
+    const char* argv[1 + DEXOPT_PARAM_COUNT + 1];
+    CHECK_EQ(static_cast<size_t>(argc), DEXOPT_PARAM_COUNT + 1);
     argv[0] = "/system/bin/otapreopt";
-    for (size_t i = 1; i <= 9; ++i) {
+    for (size_t i = 1; i <= DEXOPT_PARAM_COUNT; ++i) {
         argv[i] = arg[i];
     }
-    argv[10] = nullptr;
+    argv[DEXOPT_PARAM_COUNT + 1] = nullptr;
 
     execv(argv[0], (char * const *)argv);
     PLOG(ERROR) << "execv(OTAPREOPT) failed.";