Merge "Fix FenceTracker releaseFence"
diff --git a/cmds/atrace/Android.bp b/cmds/atrace/Android.bp
index 194a565..69ed416 100644
--- a/cmds/atrace/Android.bp
+++ b/cmds/atrace/Android.bp
@@ -6,6 +6,10 @@
 
     shared_libs: [
         "libbinder",
+        "libhwbinder",
+        "android.hidl.manager@1.0",
+        "libhidlbase",
+        "libhidltransport",
         "liblog",
         "libcutils",
         "libutils",
diff --git a/cmds/atrace/atrace.cpp b/cmds/atrace/atrace.cpp
index 8faf276..a217c5d 100644
--- a/cmds/atrace/atrace.cpp
+++ b/cmds/atrace/atrace.cpp
@@ -37,6 +37,8 @@
 #include <binder/IServiceManager.h>
 #include <binder/Parcel.h>
 
+#include <android/hidl/manager/1.0/IServiceManager.h>
+#include <hidl/ServiceManagement.h>
 #include <cutils/properties.h>
 
 #include <utils/String8.h>
@@ -47,6 +49,7 @@
 
 using namespace android;
 
+using std::string;
 #define NELEM(x) ((int) (sizeof(x) / sizeof((x)[0])))
 
 #define MAX_SYS_FILES 10
@@ -517,6 +520,35 @@
     return true;
 }
 
+// Poke all the HAL processes in the system to get them to re-read
+// their system properties.
+static void pokeHalServices()
+{
+    using ::android::hidl::manager::V1_0::IServiceManager;
+    using ::android::hardware::IBinder;
+    using ::android::hardware::hidl_string;
+    using ::android::hardware::Parcel;
+
+    Parcel data;
+
+    sp<IServiceManager> sm = ::android::hardware::defaultServiceManager();
+    sm->list([&](const auto &interfaces) {
+        for (size_t i = 0; i < interfaces.size(); i++) {
+            string fqInstanceName = interfaces[i];
+            string::size_type n = fqInstanceName.find("/");
+            if (n == std::string::npos || interfaces[i].size() == n+1)
+                continue;
+            hidl_string fqInterfaceName = fqInstanceName.substr(0, n);
+            hidl_string instanceName = fqInstanceName.substr(n+1, std::string::npos);
+            sm->get(fqInterfaceName, instanceName, [&](const auto &interface) {
+                // TODO(b/32756130)
+                // Once IServiceManager returns IBase, use interface->notifySyspropsChanged() here
+                interface->transact(IBinder::SYSPROPS_TRANSACTION, data, nullptr, 0, nullptr);
+            });
+        }
+    });
+}
+
 // Set the trace tags that userland tracing uses, and poke the running
 // processes to pick up the new value.
 static bool setTagsProperty(uint64_t tags)
@@ -759,6 +791,7 @@
     }
     ok &= setAppCmdlineProperty(&packageList[0]);
     ok &= pokeBinderServices();
+    pokeHalServices();
 
     // Disable all the sysfs enables.  This is done as a separate loop from
     // the enables to allow the same enable to exist in multiple categories.
diff --git a/cmds/atrace/atrace.rc b/cmds/atrace/atrace.rc
index d486a3d..54ba5ca 100644
--- a/cmds/atrace/atrace.rc
+++ b/cmds/atrace/atrace.rc
@@ -1,6 +1,6 @@
 ## Permissions to allow system-wide tracing to the kernel trace buffer.
 ##
-on fs
+on post-fs
 
 # Allow writing to the kernel trace log.
     chmod 0222 /sys/kernel/debug/tracing/trace_marker
diff --git a/cmds/dumpstate/Android.mk b/cmds/dumpstate/Android.mk
index e11bf30..2b23a3d 100644
--- a/cmds/dumpstate/Android.mk
+++ b/cmds/dumpstate/Android.mk
@@ -12,8 +12,10 @@
 COMMON_LOCAL_CFLAGS := \
        -Wall -Werror -Wno-missing-field-initializers -Wno-unused-variable -Wunused-parameter
 COMMON_SRC_FILES := \
+        DumpstateInternal.cpp \
         utils.cpp
 COMMON_SHARED_LIBRARIES := \
+        android.hardware.dumpstate@1.0 \
         libbase \
         libbinder \
         libcutils \
@@ -22,6 +24,27 @@
         liblog \
         libselinux \
         libutils
+COMMON_STATIC_LIBRARIES := \
+        libdumpstateutil \
+        $(COMMON_ZIP_LIBRARIES)
+
+# ====================#
+# libdumpstateutil #
+# ====================#
+include $(CLEAR_VARS)
+
+LOCAL_MODULE := libdumpstateutil
+
+LOCAL_CFLAGS := $(COMMON_LOCAL_CFLAGS)
+LOCAL_C_INCLUDES := $(LOCAL_PATH)
+LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)
+LOCAL_SRC_FILES := \
+        DumpstateInternal.cpp \
+        DumpstateUtil.cpp
+LOCAL_SHARED_LIBRARIES := \
+        libbase
+
+include $(BUILD_STATIC_LIBRARY)
 
 # ====================#
 # libdumpstateheaders #
@@ -35,7 +58,7 @@
 LOCAL_EXPORT_SHARED_LIBRARY_HEADERS := \
         $(COMMON_SHARED_LIBRARIES)
 LOCAL_EXPORT_STATIC_LIBRARY_HEADERS := \
-        $(COMMON_ZIP_LIBRARIES)
+        $(COMMON_STATIC_LIBRARIES)
 # Soong requires that whats is on LOCAL_EXPORTED_ is also on LOCAL_
 LOCAL_SHARED_LIBRARIES := $(LOCAL_EXPORT_SHARED_LIBRARY_HEADERS)
 LOCAL_STATIC_LIBRARIES := $(LOCAL_EXPORT_STATIC_LIBRARY_HEADERS)
@@ -81,7 +104,7 @@
 
 LOCAL_SHARED_LIBRARIES := $(COMMON_SHARED_LIBRARIES)
 
-LOCAL_STATIC_LIBRARIES := $(COMMON_ZIP_LIBRARIES)
+LOCAL_STATIC_LIBRARIES := $(COMMON_STATIC_LIBRARIES)
 
 LOCAL_HAL_STATIC_LIBRARIES := libdumpstate
 
@@ -106,7 +129,7 @@
         DumpstateService.cpp \
         tests/dumpstate_test.cpp
 
-LOCAL_STATIC_LIBRARIES := $(COMMON_ZIP_LIBRARIES) \
+LOCAL_STATIC_LIBRARIES := $(COMMON_STATIC_LIBRARIES) \
         libgmock
 
 LOCAL_SHARED_LIBRARIES := $(COMMON_SHARED_LIBRARIES)
diff --git a/cmds/dumpstate/DumpstateInternal.cpp b/cmds/dumpstate/DumpstateInternal.cpp
new file mode 100644
index 0000000..2d74377
--- /dev/null
+++ b/cmds/dumpstate/DumpstateInternal.cpp
@@ -0,0 +1,162 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define LOG_TAG "dumpstate"
+
+#include "DumpstateInternal.h"
+
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/capability.h>
+#include <sys/prctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+#include <cstdint>
+#include <string>
+#include <vector>
+
+#include <android-base/file.h>
+#include <cutils/log.h>
+#include <private/android_filesystem_config.h>
+
+uint64_t Nanotime() {
+    timespec ts;
+    clock_gettime(CLOCK_MONOTONIC, &ts);
+    return static_cast<uint64_t>(ts.tv_sec * NANOS_PER_SEC + ts.tv_nsec);
+}
+
+// Switches to non-root user and group.
+bool DropRootUser() {
+    if (getgid() == AID_SHELL && getuid() == AID_SHELL) {
+        MYLOGD("drop_root_user(): already running as Shell\n");
+        return true;
+    }
+    /* ensure we will keep capabilities when we drop root */
+    if (prctl(PR_SET_KEEPCAPS, 1) < 0) {
+        MYLOGE("prctl(PR_SET_KEEPCAPS) failed: %s\n", strerror(errno));
+        return false;
+    }
+
+    gid_t groups[] = {AID_LOG,          AID_SDCARD_R, AID_SDCARD_RW, AID_MOUNT,    AID_INET,
+                      AID_NET_BW_STATS, AID_READPROC, AID_WAKELOCK,  AID_BLUETOOTH};
+    if (setgroups(sizeof(groups) / sizeof(groups[0]), groups) != 0) {
+        MYLOGE("Unable to setgroups, aborting: %s\n", strerror(errno));
+        return false;
+    }
+    if (setgid(AID_SHELL) != 0) {
+        MYLOGE("Unable to setgid, aborting: %s\n", strerror(errno));
+        return false;
+    }
+    if (setuid(AID_SHELL) != 0) {
+        MYLOGE("Unable to setuid, aborting: %s\n", strerror(errno));
+        return false;
+    }
+
+    struct __user_cap_header_struct capheader;
+    struct __user_cap_data_struct capdata[2];
+    memset(&capheader, 0, sizeof(capheader));
+    memset(&capdata, 0, sizeof(capdata));
+    capheader.version = _LINUX_CAPABILITY_VERSION_3;
+    capheader.pid = 0;
+
+    capdata[CAP_TO_INDEX(CAP_SYSLOG)].permitted =
+        (CAP_TO_MASK(CAP_SYSLOG) | CAP_TO_MASK(CAP_BLOCK_SUSPEND));
+    capdata[CAP_TO_INDEX(CAP_SYSLOG)].effective =
+        (CAP_TO_MASK(CAP_SYSLOG) | CAP_TO_MASK(CAP_BLOCK_SUSPEND));
+    capdata[0].inheritable = 0;
+    capdata[1].inheritable = 0;
+
+    if (capset(&capheader, &capdata[0]) < 0) {
+        MYLOGE("capset failed: %s\n", strerror(errno));
+        return false;
+    }
+
+    return true;
+}
+
+int DumpFileFromFdToFd(const std::string& title, const std::string& path_string, int fd, int out_fd,
+                       bool dry_run) {
+    const char* path = path_string.c_str();
+    if (!title.empty()) {
+        dprintf(out_fd, "------ %s (%s", title.c_str(), path);
+
+        struct stat st;
+        // Only show the modification time of non-device files.
+        size_t path_len = strlen(path);
+        if ((path_len < 6 || memcmp(path, "/proc/", 6)) &&
+            (path_len < 5 || memcmp(path, "/sys/", 5)) &&
+            (path_len < 3 || memcmp(path, "/d/", 3)) && !fstat(fd, &st)) {
+            char stamp[80];
+            time_t mtime = st.st_mtime;
+            strftime(stamp, sizeof(stamp), "%Y-%m-%d %H:%M:%S", localtime(&mtime));
+            dprintf(out_fd, ": %s", stamp);
+        }
+        dprintf(out_fd, ") ------\n");
+        fsync(out_fd);
+    }
+    if (dry_run) {
+        if (out_fd != STDOUT_FILENO) {
+            // There is no title, but we should still print a dry-run message
+            dprintf(out_fd, "%s: skipped on dry run\n", path);
+        } else if (!title.empty()) {
+            dprintf(out_fd, "\t(skipped on dry run)\n");
+        }
+        fsync(out_fd);
+        return 0;
+    }
+    bool newline = false;
+    fd_set read_set;
+    timeval tm;
+    while (true) {
+        FD_ZERO(&read_set);
+        FD_SET(fd, &read_set);
+        /* Timeout if no data is read for 30 seconds. */
+        tm.tv_sec = 30;
+        tm.tv_usec = 0;
+        uint64_t elapsed = Nanotime();
+        int ret = TEMP_FAILURE_RETRY(select(fd + 1, &read_set, nullptr, nullptr, &tm));
+        if (ret == -1) {
+            dprintf(out_fd, "*** %s: select failed: %s\n", path, strerror(errno));
+            newline = true;
+            break;
+        } else if (ret == 0) {
+            elapsed = Nanotime() - elapsed;
+            dprintf(out_fd, "*** %s: Timed out after %.3fs\n", path, (float)elapsed / NANOS_PER_SEC);
+            newline = true;
+            break;
+        } else {
+            char buffer[65536];
+            ssize_t bytes_read = TEMP_FAILURE_RETRY(read(fd, buffer, sizeof(buffer)));
+            if (bytes_read > 0) {
+                android::base::WriteFully(out_fd, buffer, bytes_read);
+                newline = (buffer[bytes_read - 1] == '\n');
+            } else {
+                if (bytes_read == -1) {
+                    dprintf(out_fd, "*** %s: Failed to read from fd: %s", path, strerror(errno));
+                    newline = true;
+                }
+                break;
+            }
+        }
+    }
+    close(fd);
+
+    if (!newline) dprintf(out_fd, "\n");
+    if (!title.empty()) dprintf(out_fd, "\n");
+    return 0;
+}
diff --git a/cmds/dumpstate/DumpstateInternal.h b/cmds/dumpstate/DumpstateInternal.h
new file mode 100644
index 0000000..2f7704d
--- /dev/null
+++ b/cmds/dumpstate/DumpstateInternal.h
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef FRAMEWORK_NATIVE_CMD_DUMPSTATE_INTERNAL_H_
+#define FRAMEWORK_NATIVE_CMD_DUMPSTATE_INTERNAL_H_
+
+#include <cstdint>
+#include <string>
+
+// TODO: rename macros to DUMPSTATE_LOGXXX
+#ifndef MYLOGD
+#define MYLOGD(...)               \
+    fprintf(stderr, __VA_ARGS__); \
+    ALOGD(__VA_ARGS__);
+#endif
+
+#ifndef MYLOGI
+#define MYLOGI(...)               \
+    fprintf(stderr, __VA_ARGS__); \
+    ALOGI(__VA_ARGS__);
+#endif
+
+#ifndef MYLOGE
+#define MYLOGE(...)               \
+    fprintf(stderr, __VA_ARGS__); \
+    ALOGE(__VA_ARGS__);
+#endif
+
+// Internal functions used by .cpp files on multiple build targets.
+// TODO: move to android::os::dumpstate::internal namespace
+
+// TODO: use functions from <chrono> instead
+const uint64_t NANOS_PER_SEC = 1000000000;
+uint64_t Nanotime();
+
+// Switches to non-root user and group.
+bool DropRootUser();
+
+// TODO: move to .cpp as static once is not used by utils.cpp anymore.
+int DumpFileFromFdToFd(const std::string& title, const std::string& path_string, int fd, int out_fd,
+                       bool dry_run = false);
+
+#endif  // FRAMEWORK_NATIVE_CMD_DUMPSTATE_INTERNAL_H_
diff --git a/cmds/dumpstate/DumpstateService.cpp b/cmds/dumpstate/DumpstateService.cpp
index f2dcf2b..5430956 100644
--- a/cmds/dumpstate/DumpstateService.cpp
+++ b/cmds/dumpstate/DumpstateService.cpp
@@ -22,6 +22,8 @@
 
 #include "android/os/BnDumpstate.h"
 
+#include "DumpstateInternal.h"
+
 namespace android {
 namespace os {
 
diff --git a/cmds/dumpstate/DumpstateUtil.cpp b/cmds/dumpstate/DumpstateUtil.cpp
new file mode 100644
index 0000000..9faa63e
--- /dev/null
+++ b/cmds/dumpstate/DumpstateUtil.cpp
@@ -0,0 +1,344 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define LOG_TAG "dumpstate"
+
+#include "DumpstateUtil.h"
+
+#include <fcntl.h>
+#include <sys/prctl.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <vector>
+
+#include <android-base/properties.h>
+#include <cutils/log.h>
+
+#include "DumpstateInternal.h"
+
+// TODO: move to unnamed namespace
+static constexpr const char* kSuPath = "/system/xbin/su";
+
+// TODO: move to unnamed namespace
+static bool waitpid_with_timeout(pid_t pid, int timeout_seconds, int* status) {
+    sigset_t child_mask, old_mask;
+    sigemptyset(&child_mask);
+    sigaddset(&child_mask, SIGCHLD);
+
+    if (sigprocmask(SIG_BLOCK, &child_mask, &old_mask) == -1) {
+        printf("*** sigprocmask failed: %s\n", strerror(errno));
+        return false;
+    }
+
+    timespec ts;
+    ts.tv_sec = timeout_seconds;
+    ts.tv_nsec = 0;
+    int ret = TEMP_FAILURE_RETRY(sigtimedwait(&child_mask, NULL, &ts));
+    int saved_errno = errno;
+    // Set the signals back the way they were.
+    if (sigprocmask(SIG_SETMASK, &old_mask, NULL) == -1) {
+        printf("*** sigprocmask failed: %s\n", strerror(errno));
+        if (ret == 0) {
+            return false;
+        }
+    }
+    if (ret == -1) {
+        errno = saved_errno;
+        if (errno == EAGAIN) {
+            errno = ETIMEDOUT;
+        } else {
+            printf("*** sigtimedwait failed: %s\n", strerror(errno));
+        }
+        return false;
+    }
+
+    pid_t child_pid = waitpid(pid, status, WNOHANG);
+    if (child_pid != pid) {
+        if (child_pid != -1) {
+            printf("*** Waiting for pid %d, got pid %d instead\n", pid, child_pid);
+        } else {
+            printf("*** waitpid failed: %s\n", strerror(errno));
+        }
+        return false;
+    }
+    return true;
+}
+
+CommandOptions CommandOptions::DEFAULT = CommandOptions::WithTimeout(10).Build();
+CommandOptions CommandOptions::AS_ROOT = CommandOptions::WithTimeout(10).AsRoot().Build();
+CommandOptions CommandOptions::AS_ROOT_5 = CommandOptions::WithTimeout(5).AsRoot().Build();
+
+CommandOptions::CommandOptionsBuilder::CommandOptionsBuilder(int64_t timeout) : values(timeout) {
+}
+
+CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::Always() {
+    values.always_ = true;
+    return *this;
+}
+
+CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::AsRoot() {
+    values.account_mode_ = SU_ROOT;
+    return *this;
+}
+
+CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::DropRoot() {
+    values.account_mode_ = DROP_ROOT;
+    return *this;
+}
+
+CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::RedirectStderr() {
+    values.output_mode_ = REDIRECT_TO_STDERR;
+    return *this;
+}
+
+CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::Log(
+    const std::string& message) {
+    values.logging_message_ = message;
+    return *this;
+}
+
+CommandOptions CommandOptions::CommandOptionsBuilder::Build() {
+    return CommandOptions(values);
+}
+
+CommandOptions::CommandOptionsValues::CommandOptionsValues(int64_t timeout)
+    : timeout_(timeout),
+      always_(false),
+      account_mode_(DONT_DROP_ROOT),
+      output_mode_(NORMAL_OUTPUT),
+      logging_message_("") {
+}
+
+CommandOptions::CommandOptions(const CommandOptionsValues& values) : values(values) {
+}
+
+int64_t CommandOptions::Timeout() const {
+    return values.timeout_;
+}
+
+bool CommandOptions::Always() const {
+    return values.always_;
+}
+
+PrivilegeMode CommandOptions::PrivilegeMode() const {
+    return values.account_mode_;
+}
+
+OutputMode CommandOptions::OutputMode() const {
+    return values.output_mode_;
+}
+
+std::string CommandOptions::LoggingMessage() const {
+    return values.logging_message_;
+}
+
+CommandOptions::CommandOptionsBuilder CommandOptions::WithTimeout(int64_t timeout) {
+    return CommandOptions::CommandOptionsBuilder(timeout);
+}
+
+std::string PropertiesHelper::build_type_ = "";
+int PropertiesHelper::dry_run_ = -1;
+
+bool PropertiesHelper::IsUserBuild() {
+    if (build_type_.empty()) {
+        build_type_ = android::base::GetProperty("ro.build.type", "user");
+    }
+    return "user" == build_type_;
+}
+
+bool PropertiesHelper::IsDryRun() {
+    if (dry_run_ == -1) {
+        dry_run_ = android::base::GetBoolProperty("dumpstate.dry_run", false) ? 1 : 0;
+    }
+    return dry_run_ == 1;
+}
+
+int DumpFileToFd(int out_fd, const std::string& title, const std::string& path) {
+    int fd = TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY | O_NONBLOCK | O_CLOEXEC));
+    if (fd < 0) {
+        int err = errno;
+        if (title.empty()) {
+            dprintf(out_fd, "*** Error dumping %s: %s\n", path.c_str(), strerror(err));
+        } else {
+            dprintf(out_fd, "*** Error dumping %s (%s): %s\n", path.c_str(), title.c_str(),
+                    strerror(err));
+        }
+        fsync(out_fd);
+        return -1;
+    }
+    return DumpFileFromFdToFd(title, path, fd, out_fd, PropertiesHelper::IsDryRun());
+}
+
+int RunCommandToFd(int fd, const std::string& title, const std::vector<std::string>& full_command,
+                   const CommandOptions& options) {
+    if (full_command.empty()) {
+        MYLOGE("No arguments on RunCommandToFd(%s)\n", title.c_str());
+        return -1;
+    }
+
+    int size = full_command.size() + 1;  // null terminated
+    int starting_index = 0;
+    if (options.PrivilegeMode() == SU_ROOT) {
+        starting_index = 2;  // "su" "root"
+        size += starting_index;
+    }
+
+    std::vector<const char*> args;
+    args.resize(size);
+
+    std::string command_string;
+    if (options.PrivilegeMode() == SU_ROOT) {
+        args[0] = kSuPath;
+        command_string += kSuPath;
+        args[1] = "root";
+        command_string += " root ";
+    }
+    for (size_t i = 0; i < full_command.size(); i++) {
+        args[i + starting_index] = full_command[i].data();
+        command_string += args[i + starting_index];
+        if (i != full_command.size() - 1) {
+            command_string += " ";
+        }
+    }
+    args[size - 1] = nullptr;
+
+    const char* command = command_string.c_str();
+
+    if (options.PrivilegeMode() == SU_ROOT && PropertiesHelper::IsUserBuild()) {
+        dprintf(fd, "Skipping '%s' on user build.\n", command);
+        return 0;
+    }
+
+    if (!title.empty()) {
+        dprintf(fd, "------ %s (%s) ------\n", title.c_str(), command);
+        fsync(fd);
+    }
+
+    const std::string& logging_message = options.LoggingMessage();
+    if (!logging_message.empty()) {
+        MYLOGI(logging_message.c_str(), command_string.c_str());
+    }
+
+    bool silent = (options.OutputMode() == REDIRECT_TO_STDERR);
+    bool redirecting_to_fd = STDOUT_FILENO != fd;
+
+    if (PropertiesHelper::IsDryRun() && !options.Always()) {
+        if (!title.empty()) {
+            dprintf(fd, "\t(skipped on dry run)\n");
+        } else if (redirecting_to_fd) {
+            // There is no title, but we should still print a dry-run message
+            dprintf(fd, "%s: skipped on dry run\n", command_string.c_str());
+        }
+        fsync(fd);
+        return 0;
+    }
+
+    const char* path = args[0];
+
+    uint64_t start = Nanotime();
+    pid_t pid = fork();
+
+    /* handle error case */
+    if (pid < 0) {
+        if (!silent) dprintf(fd, "*** fork: %s\n", strerror(errno));
+        MYLOGE("*** fork: %s\n", strerror(errno));
+        return pid;
+    }
+
+    /* handle child case */
+    if (pid == 0) {
+        if (options.PrivilegeMode() == DROP_ROOT && !DropRootUser()) {
+            if (!silent) {
+                dprintf(fd, "*** failed to drop root before running %s: %s\n", command,
+                        strerror(errno));
+            }
+            MYLOGE("*** could not drop root before running %s: %s\n", command, strerror(errno));
+            return -1;
+        }
+
+        if (silent) {
+            // Redirects stdout to stderr
+            TEMP_FAILURE_RETRY(dup2(STDERR_FILENO, STDOUT_FILENO));
+        } else if (redirecting_to_fd) {
+            // Redirect stdout to fd
+            TEMP_FAILURE_RETRY(dup2(fd, STDOUT_FILENO));
+            close(fd);
+        }
+
+        /* make sure the child dies when dumpstate dies */
+        prctl(PR_SET_PDEATHSIG, SIGKILL);
+
+        /* just ignore SIGPIPE, will go down with parent's */
+        struct sigaction sigact;
+        memset(&sigact, 0, sizeof(sigact));
+        sigact.sa_handler = SIG_IGN;
+        sigaction(SIGPIPE, &sigact, NULL);
+
+        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));
+        // Must call _exit (instead of exit), otherwise it will corrupt the zip
+        // file.
+        _exit(EXIT_FAILURE);
+    }
+
+    /* handle parent case */
+    int status;
+    bool ret = waitpid_with_timeout(pid, options.Timeout(), &status);
+    fsync(fd);
+
+    uint64_t elapsed = Nanotime() - start;
+    if (!ret) {
+        if (errno == ETIMEDOUT) {
+            if (!silent)
+                dprintf(fd, "*** command '%s' timed out after %.3fs (killing pid %d)\n", command,
+                        static_cast<float>(elapsed) / NANOS_PER_SEC, pid);
+            MYLOGE("*** command '%s' timed out after %.3fs (killing pid %d)\n", command,
+                   static_cast<float>(elapsed) / NANOS_PER_SEC, pid);
+        } else {
+            if (!silent)
+                dprintf(fd, "*** command '%s': Error after %.4fs (killing pid %d)\n", command,
+                        static_cast<float>(elapsed) / NANOS_PER_SEC, pid);
+            MYLOGE("command '%s': Error after %.4fs (killing pid %d)\n", command,
+                   static_cast<float>(elapsed) / NANOS_PER_SEC, pid);
+        }
+        kill(pid, SIGTERM);
+        if (!waitpid_with_timeout(pid, 5, nullptr)) {
+            kill(pid, SIGKILL);
+            if (!waitpid_with_timeout(pid, 5, nullptr)) {
+                if (!silent)
+                    dprintf(fd, "could not kill command '%s' (pid %d) even with SIGKILL.\n",
+                            command, pid);
+                MYLOGE("could not kill command '%s' (pid %d) even with SIGKILL.\n", command, pid);
+            }
+        }
+        return -1;
+    }
+
+    if (WIFSIGNALED(status)) {
+        if (!silent)
+            dprintf(fd, "*** command '%s' failed: killed by signal %d\n", command, WTERMSIG(status));
+        MYLOGE("*** command '%s' failed: killed by signal %d\n", command, WTERMSIG(status));
+    } else if (WIFEXITED(status) && WEXITSTATUS(status) > 0) {
+        status = WEXITSTATUS(status);
+        if (!silent) dprintf(fd, "*** command '%s' failed: exit code %d\n", command, status);
+        MYLOGE("*** command '%s' failed: exit code %d\n", command, status);
+    }
+
+    return status;
+}
diff --git a/cmds/dumpstate/DumpstateUtil.h b/cmds/dumpstate/DumpstateUtil.h
new file mode 100644
index 0000000..8bda6f2
--- /dev/null
+++ b/cmds/dumpstate/DumpstateUtil.h
@@ -0,0 +1,177 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef FRAMEWORK_NATIVE_CMD_DUMPSTATE_UTIL_H_
+#define FRAMEWORK_NATIVE_CMD_DUMPSTATE_UTIL_H_
+
+#include <cstdint>
+#include <string>
+
+// TODO: use android::os::dumpstate (must wait until device code is refactored)
+
+/*
+ * Defines the Linux account that should be executing a command.
+ */
+enum PrivilegeMode {
+    /* Explicitly change the `uid` and `gid` to be `shell`.*/
+    DROP_ROOT,
+    /* Don't change the `uid` and `gid`. */
+    DONT_DROP_ROOT,
+    /* Prefix the command with `/PATH/TO/su root`. Won't work non user builds. */
+    SU_ROOT
+};
+
+/*
+ * Defines what should happen with the main output stream (`stdout` or fd) of a command.
+ */
+enum OutputMode {
+    /* Don't change main output. */
+    NORMAL_OUTPUT,
+    /* Redirect main output to `stderr`. */
+    REDIRECT_TO_STDERR
+};
+
+/*
+ * Value object used to set command options.
+ *
+ * Typically constructed using a builder with chained setters. Examples:
+ *
+ *  CommandOptions::WithTimeout(20).AsRoot().Build();
+ *  CommandOptions::WithTimeout(10).Always().RedirectStderr().Build();
+ *
+ * Although the builder could be used to dynamically set values. Example:
+ *
+ *  CommandOptions::CommandOptionsBuilder options =
+ *  CommandOptions::WithTimeout(10);
+ *  if (!is_user_build()) {
+ *    options.AsRoot();
+ *  }
+ *  RunCommand("command", {"args"}, options.Build());
+ */
+class CommandOptions {
+  private:
+    class CommandOptionsValues {
+      private:
+        CommandOptionsValues(int64_t timeout);
+
+        int64_t timeout_;
+        bool always_;
+        PrivilegeMode account_mode_;
+        OutputMode output_mode_;
+        std::string logging_message_;
+
+        friend class CommandOptions;
+        friend class CommandOptionsBuilder;
+    };
+
+    CommandOptions(const CommandOptionsValues& values);
+
+    const CommandOptionsValues values;
+
+  public:
+    class CommandOptionsBuilder {
+      public:
+        /* Sets the command to always run, even on `dry-run` mode. */
+        CommandOptionsBuilder& Always();
+        /* Sets the command's PrivilegeMode as `SU_ROOT` */
+        CommandOptionsBuilder& AsRoot();
+        /* Sets the command's PrivilegeMode as `DROP_ROOT` */
+        CommandOptionsBuilder& DropRoot();
+        /* Sets the command's OutputMode as `REDIRECT_TO_STDERR` */
+        CommandOptionsBuilder& RedirectStderr();
+        /* When not empty, logs a message before executing the command.
+         * Must contain a `%s`, which will be replaced by the full command line, and end on `\n`. */
+        CommandOptionsBuilder& Log(const std::string& message);
+        /* Builds the command options. */
+        CommandOptions Build();
+
+      private:
+        CommandOptionsBuilder(int64_t timeout);
+        CommandOptionsValues values;
+        friend class CommandOptions;
+    };
+
+    /** Gets the command timeout, in seconds. */
+    int64_t Timeout() const;
+    /* Checks whether the command should always be run, even on dry-run mode. */
+    bool Always() const;
+    /** Gets the PrivilegeMode of the command. */
+    PrivilegeMode PrivilegeMode() const;
+    /** Gets the OutputMode of the command. */
+    OutputMode OutputMode() const;
+    /** Gets the logging message header, it any. */
+    std::string LoggingMessage() const;
+
+    /** Creates a builder with the requied timeout. */
+    static CommandOptionsBuilder WithTimeout(int64_t timeout);
+
+    // Common options.
+    static CommandOptions DEFAULT;
+    static CommandOptions AS_ROOT;
+
+    // TODO: temporary, until device implementations use AS_ROOT
+    static CommandOptions AS_ROOT_5;
+};
+
+/*
+ * System properties helper.
+ */
+class PropertiesHelper {
+    friend class DumpstateBaseTest;
+
+  public:
+    /*
+     * Gets whether device is running a `user` build.
+     */
+    static bool IsUserBuild();
+
+    /*
+     * When running in dry-run mode, skips the real dumps and just print the section headers.
+     *
+     * Useful when debugging dumpstate or other bugreport-related activities.
+     *
+     * Dry-run mode is enabled by setting the system property `dumpstate.dry_run` to true.
+     */
+    static bool IsDryRun();
+
+  private:
+    static std::string build_type_;
+    static int dry_run_;
+};
+
+/*
+ * Forks a command, waits for it to finish, and returns its status.
+ *
+ * |fd| file descriptor that receives the command's 'stdout'.
+ * |title| description of the command printed on `stdout` (or empty to skip
+ * description).
+ * |full_command| array containing the command (first entry) and its arguments.
+ *                Must contain at least one element.
+ * |options| optional argument defining the command's behavior.
+ */
+int RunCommandToFd(int fd, const std::string& title, const std::vector<std::string>& full_command,
+                   const CommandOptions& options = CommandOptions::DEFAULT);
+
+/*
+ * Dumps the contents of a file into a file descriptor.
+ *
+ * |fd| file descriptor where the file is dumped into.
+ * |title| description of the command printed on `stdout` (or empty to skip
+ * description).
+ * |path| location of the file to be dumped.
+ */
+int DumpFileToFd(int fd, const std::string& title, const std::string& path);
+
+#endif  // FRAMEWORK_NATIVE_CMD_DUMPSTATE_UTIL_H_
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 6201735..30b5d9a 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 #define LOG_TAG "dumpstate"
 
 #include <dirent.h>
@@ -41,12 +42,15 @@
 #include <android-base/stringprintf.h>
 #include <android-base/strings.h>
 #include <android-base/unique_fd.h>
+#include <android/hardware/dumpstate/1.0/IDumpstateDevice.h>
+#include <cutils/native_handle.h>
 #include <cutils/properties.h>
 #include <hardware_legacy/power.h>
 #include <openssl/sha.h>
 #include <private/android_filesystem_config.h>
 #include <private/android_logger.h>
 
+#include "DumpstateInternal.h"
 #include "DumpstateService.h"
 #include "dumpstate.h"
 
@@ -90,16 +94,13 @@
     return ds.RunCommand(title, fullCommand, options);
 }
 static void RunDumpsys(const std::string& title, const std::vector<std::string>& dumpsysArgs,
-                       const CommandOptions& options = CommandOptions::DEFAULT_DUMPSYS,
+                       const CommandOptions& options = Dumpstate::DEFAULT_DUMPSYS,
                        long dumpsysTimeout = 0) {
     return ds.RunDumpsys(title, dumpsysArgs, options, dumpsysTimeout);
 }
 static int DumpFile(const std::string& title, const std::string& path) {
     return ds.DumpFile(title, path);
 }
-bool IsUserBuild() {
-    return ds.IsUserBuild();
-}
 
 // Relative directory (inside the zip) for all files copied as-is into the bugreport.
 static const std::string ZIP_ROOT_DIR = "FS";
@@ -108,6 +109,8 @@
 static constexpr char PROPERTY_LAST_ID[] = "dumpstate.last_id";
 static constexpr char PROPERTY_VERSION[] = "dumpstate.version";
 
+static const CommandOptions AS_ROOT_20 = CommandOptions::WithTimeout(20).AsRoot().Build();
+
 /* gets the tombstone data, according to the bugreport type: if zipped, gets all tombstones;
  * otherwise, gets just those modified in the last half an hour. */
 static void get_tombstone_fds(tombstone_data_t data[NUM_TOMBSTONES]) {
@@ -157,7 +160,7 @@
     if (!ds.IsZipping()) return;
     std::string title = "MOUNT INFO";
     mount_points.clear();
-    DurationReporter duration_reporter(title, nullptr);
+    DurationReporter duration_reporter(title, true);
     for_each_pid(do_mountinfo, nullptr);
     MYLOGD("%s: %d entries added to zip file\n", title.c_str(), (int)mount_points.size());
 }
@@ -357,7 +360,7 @@
 }
 
 static void dump_raft() {
-    if (IsUserBuild()) {
+    if (PropertiesHelper::IsUserBuild()) {
         return;
     }
 
@@ -428,7 +431,7 @@
 
 static void DumpModemLogs() {
     DurationReporter durationReporter("DUMP MODEM LOGS");
-    if (IsUserBuild()) {
+    if (PropertiesHelper::IsUserBuild()) {
         return;
     }
 
@@ -462,6 +465,9 @@
         MYLOGE("Unable to add modem log %s to zip file\n", modem_log_file.c_str());
     } else {
         MYLOGD("Modem Log %s is added to zip\n", modem_log_file.c_str());
+        if (remove(modem_log_file.c_str())) {
+            MYLOGE("Error removing modem log %s\n", modem_log_file.c_str());
+        }
     }
 }
 
@@ -680,12 +686,14 @@
     printf("Network: %s\n", network.c_str());
 
     printf("Kernel: ");
-    JustDumpFile("", "/proc/version");
+    fflush(stdout);
+    DumpFileToFd(STDOUT_FILENO, "", "/proc/version");
     printf("Command line: %s\n", strtok(cmdline_buf, "\n"));
     printf("Bugreport format version: %s\n", version_.c_str());
     printf("Dumpstate info: id=%d pid=%d dry_run=%d args=%s extra_options=%s\n", id_, pid_,
-           dry_run_, args_.c_str(), extra_options_.c_str());
+           PropertiesHelper::IsDryRun(), args_.c_str(), extra_options_.c_str());
     printf("\n");
+    fflush(stdout);
 }
 
 // List of file extensions that can cause a zip file attachment to be rejected by some email
@@ -772,7 +780,7 @@
         return;
     }
     MYLOGD("Adding dir %s (recursive: %d)\n", dir.c_str(), recursive);
-    DurationReporter duration_reporter(dir, nullptr);
+    DurationReporter duration_reporter(dir, true);
     dump_files("", dir.c_str(), recursive ? skip_none : is_dir, _add_file_from_fd);
 }
 
@@ -909,7 +917,7 @@
     DumpFile("MEMORY INFO", "/proc/meminfo");
     RunCommand("CPU INFO", {"top", "-b", "-n", "1", "-H", "-s", "6", "-o",
                             "pid,tid,user,pr,ni,%cpu,s,virt,res,pcy,cmd,name"});
-    RunCommand("PROCRANK", {"procrank"}, CommandOptions::AS_ROOT_20);
+    RunCommand("PROCRANK", {"procrank"}, AS_ROOT_20);
     DumpFile("VIRTUAL MEMORY STATS", "/proc/vmstat");
     DumpFile("VMALLOC INFO", "/proc/vmallocinfo");
     DumpFile("SLAB INFO", "/proc/slabinfo");
@@ -924,7 +932,7 @@
 
     RunCommand("PROCESSES AND THREADS",
                {"ps", "-A", "-T", "-Z", "-O", "pri,nice,rtprio,sched,pcy"});
-    RunCommand("LIBRANK", {"librank"}, CommandOptions::AS_ROOT_10);
+    RunCommand("LIBRANK", {"librank"}, CommandOptions::AS_ROOT);
 
     RunCommand("PRINTENV", {"printenv"});
     RunCommand("NETSTAT", {"netstat", "-nW"});
@@ -937,7 +945,7 @@
 
     do_dmesg();
 
-    RunCommand("LIST OF OPEN FILES", {"lsof"}, CommandOptions::AS_ROOT_10);
+    RunCommand("LIST OF OPEN FILES", {"lsof"}, CommandOptions::AS_ROOT);
     for_each_pid(do_showmap, "SMAPS OF ALL PROCESSES");
     for_each_tid(show_wchan, "BLOCKED PROCESS WAIT-CHANNELS");
     for_each_pid(show_showtime, "PROCESS TIMES (pid cmd user system iowait+percentage)");
@@ -1040,7 +1048,7 @@
 #ifdef FWDUMP_bcmdhd
     RunCommand("ND OFFLOAD TABLE", {WLUTIL, "nd_hostip"}, CommandOptions::AS_ROOT_5);
 
-    RunCommand("DUMP WIFI INTERNAL COUNTERS (1)", {WLUTIL, "counters"}, CommandOptions::AS_ROOT_20);
+    RunCommand("DUMP WIFI INTERNAL COUNTERS (1)", {WLUTIL, "counters"}, AS_ROOT_20);
 
     RunCommand("ND OFFLOAD STATUS (1)", {WLUTIL, "nd_status"}, CommandOptions::AS_ROOT_5);
 
@@ -1051,9 +1059,9 @@
                CommandOptions::WithTimeout(10).Build());
 
 #ifdef FWDUMP_bcmdhd
-    RunCommand("DUMP WIFI STATUS", {"dhdutil", "-i", "wlan0", "dump"}, CommandOptions::AS_ROOT_20);
+    RunCommand("DUMP WIFI STATUS", {"dhdutil", "-i", "wlan0", "dump"}, AS_ROOT_20);
 
-    RunCommand("DUMP WIFI INTERNAL COUNTERS (2)", {WLUTIL, "counters"}, CommandOptions::AS_ROOT_20);
+    RunCommand("DUMP WIFI INTERNAL COUNTERS (2)", {WLUTIL, "counters"}, AS_ROOT_20);
 
     RunCommand("ND OFFLOAD STATUS (2)", {WLUTIL, "nd_status"}, CommandOptions::AS_ROOT_5);
 #endif
@@ -1088,15 +1096,7 @@
     DumpFile("BINDER STATS", "/sys/kernel/debug/binder/stats");
     DumpFile("BINDER STATE", "/sys/kernel/debug/binder/state");
 
-    printf("========================================================\n");
-    printf("== Board\n");
-    printf("========================================================\n");
-
-    {
-        DurationReporter tmpDr("dumpstate_board()");
-        dumpstate_board();
-        printf("\n");
-    }
+    ds.DumpstateBoard();
 
     /* Migrate the ril_dumpstate to a dumpstate_board()? */
     int rilDumpstateTimeout = android::base::GetIntProperty("ril.dumpstate.timeout", 0);
@@ -1106,7 +1106,7 @@
         // root can run on user builds.
         CommandOptions::CommandOptionsBuilder options =
             CommandOptions::WithTimeout(rilDumpstateTimeout);
-        if (!IsUserBuild()) {
+        if (!PropertiesHelper::IsUserBuild()) {
             options.AsRoot();
         }
         RunCommand("DUMP VENDOR RIL LOGS", {"vril-dump"}, options.Build());
@@ -1161,6 +1161,60 @@
     printf("========================================================\n");
 }
 
+void Dumpstate::DumpstateBoard() {
+    DurationReporter duration_reporter("dumpstate_board()");
+    printf("========================================================\n");
+    printf("== Board\n");
+    printf("========================================================\n");
+    fflush(stdout);
+
+    android::sp<android::hardware::dumpstate::V1_0::IDumpstateDevice> dumpstate_device(
+        android::hardware::dumpstate::V1_0::IDumpstateDevice::getService("DumpstateDevice"));
+    if (dumpstate_device == nullptr) {
+        // TODO: temporary workaround until devices on master implement it
+        MYLOGE("no IDumpstateDevice implementation; using legacy dumpstate_board()\n");
+        dumpstate_board();
+        return;
+    }
+
+    if (!IsZipping()) {
+        MYLOGE("Not dumping board info because it's not a zipped bugreport\n");
+        return;
+    }
+
+    std::string path = ds.GetPath("-dumpstate-board.txt");
+    MYLOGI("Calling IDumpstateDevice implementation using path %s\n", path.c_str());
+
+    int fd =
+        TEMP_FAILURE_RETRY(open(path.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW,
+                                S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
+    if (fd < 0) {
+        MYLOGE("Could not open file %s: %s\n", path.c_str(), strerror(errno));
+        return;
+    }
+
+    native_handle_t* handle = native_handle_create(1, 0);
+    if (handle == nullptr) {
+        MYLOGE("Could not create native_handle\n");
+        return;
+    }
+    handle->data[0] = fd;
+
+    // TODO: need a timeout mechanism so dumpstate does not hang on device implementation call.
+    dumpstate_device->dumpstateBoard(handle);
+
+    AddZipEntry("dumpstate-board.txt", path);
+    printf("*** See dumpstate-board.txt entry ***\n");
+    fflush(stdout);
+
+    native_handle_close(handle);
+    native_handle_delete(handle);
+
+    if (remove(path.c_str()) != 0) {
+        MYLOGE("Could not remove(%s): %s\n", path.c_str(), strerror(errno));
+    }
+}
+
 static void ShowUsageAndExit(int exitCode = 1) {
     fprintf(stderr,
             "usage: dumpstate [-h] [-b soundfile] [-e soundfile] [-o file] [-d] [-p] "
@@ -1253,13 +1307,9 @@
     // TODO: remove once FinishZipFile() is automatically handled by Dumpstate's destructor.
     ds.zip_file.reset(nullptr);
 
-    if (IsUserBuild()) {
-        MYLOGD("Removing temporary file %s\n", tmp_path_.c_str())
-        if (remove(tmp_path_.c_str()) != 0) {
-            ALOGW("remove(%s): %s\n", tmp_path_.c_str(), strerror(errno));
-        }
-    } else {
-        MYLOGD("Keeping temporary file %s on non-user build\n", tmp_path_.c_str())
+    MYLOGD("Removing temporary file %s\n", tmp_path_.c_str())
+    if (remove(tmp_path_.c_str()) != 0) {
+        MYLOGE("Failed to remove temporary file (%s): %s\n", tmp_path_.c_str(), strerror(errno));
     }
 
     return true;
@@ -1450,7 +1500,7 @@
         }
     }
 
-    if (ds.IsDryRun()) {
+    if (PropertiesHelper::IsDryRun()) {
         MYLOGI("Running on dry-run mode (to disable it, call 'setprop dumpstate.dry_run false')\n");
     }
 
@@ -1612,7 +1662,7 @@
     ds.AddDir(RECOVERY_DIR, true);
     ds.AddDir(RECOVERY_DATA_DIR, true);
     ds.AddDir(LOGPERSIST_DATA_DIR, false);
-    if (!IsUserBuild()) {
+    if (!PropertiesHelper::IsUserBuild()) {
         ds.AddDir(PROFILE_DATA_DIR_CUR, true);
         ds.AddDir(PROFILE_DATA_DIR_REF, true);
     }
@@ -1626,7 +1676,7 @@
     // Run ss as root so we can see socket marks.
     RunCommand("DETAILED SOCKET STATE", {"ss", "-eionptu"}, CommandOptions::WithTimeout(10).Build());
 
-    if (!drop_root_user()) {
+    if (!DropRootUser()) {
         return -1;
     }
 
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index 3d3d7ed..fcf0683 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -17,18 +17,6 @@
 #ifndef FRAMEWORK_NATIVE_CMD_DUMPSTATE_H_
 #define FRAMEWORK_NATIVE_CMD_DUMPSTATE_H_
 
-#ifndef MYLOGD
-#define MYLOGD(...) fprintf(stderr, __VA_ARGS__); ALOGD(__VA_ARGS__);
-#endif
-
-#ifndef MYLOGI
-#define MYLOGI(...) fprintf(stderr, __VA_ARGS__); ALOGI(__VA_ARGS__);
-#endif
-
-#ifndef MYLOGE
-#define MYLOGE(...) fprintf(stderr, __VA_ARGS__); ALOGE(__VA_ARGS__);
-#endif
-
 #include <time.h>
 #include <unistd.h>
 #include <stdbool.h>
@@ -40,6 +28,7 @@
 #include <android-base/macros.h>
 #include <ziparchive/zip_writer.h>
 
+#include "DumpstateUtil.h"
 #include "android/os/BnDumpstate.h"
 
 // Workaround for const char *args[MAX_ARGS_ARRAY_SIZE] variables until they're converted to
@@ -53,28 +42,6 @@
 #endif
 
 /*
- * Defines the Linux user that should be executing a command.
- */
-enum RootMode {
-    /* Explicitly change the `uid` and `gid` to be `shell`.*/
-    DROP_ROOT,
-    /* Don't change the `uid` and `gid`. */
-    DONT_DROP_ROOT,
-    /* Prefix the command with `/PATH/TO/su root`. Won't work non user builds. */
-    SU_ROOT
-};
-
-/*
- * Defines what should happen with the `stdout` stream of a command.
- */
-enum StdoutMode {
-    /* Don't change `stdout`. */
-    NORMAL_STDOUT,
-    /* Redirect `stdout` to `stderr`. */
-    REDIRECT_TO_STDERR
-};
-
-/*
  * Helper class used to report how long it takes for a section to finish.
  *
  * Typical usage:
@@ -84,106 +51,19 @@
  */
 class DurationReporter {
   public:
-    DurationReporter(const std::string& title);
-    DurationReporter(const std::string& title, FILE* out);
+    DurationReporter(const std::string& title, bool log_only = false);
 
     ~DurationReporter();
 
-    static uint64_t Nanotime();
-
   private:
-    // TODO: use std::string for title, once dump_files() and other places that pass a char* are
-    // refactored as well.
     std::string title_;
-    FILE* out_;
+    bool log_only_;
     uint64_t started_;
 
     DISALLOW_COPY_AND_ASSIGN(DurationReporter);
 };
 
 /*
- * Value object used to set command options.
- *
- * Typically constructed using a builder with chained setters. Examples:
- *
- *  CommandOptions::WithTimeout(20).AsRoot().Build();
- *  CommandOptions::WithTimeout(10).Always().RedirectStderr().Build();
- *
- * Although the builder could be used to dynamically set values. Example:
- *
- *  CommandOptions::CommandOptionsBuilder options =
- *  CommandOptions::WithTimeout(10);
- *  if (!is_user_build()) {
- *    options.AsRoot();
- *  }
- *  RunCommand("command", {"args"}, options.Build());
- */
-class CommandOptions {
-  private:
-    class CommandOptionsValues {
-      private:
-        CommandOptionsValues(long timeout);
-
-        long timeout_;
-        bool always_;
-        RootMode root_mode_;
-        StdoutMode stdout_mode_;
-        std::string logging_message_;
-
-        friend class CommandOptions;
-        friend class CommandOptionsBuilder;
-    };
-
-    CommandOptions(const CommandOptionsValues& values);
-
-    const CommandOptionsValues values;
-
-  public:
-    class CommandOptionsBuilder {
-      public:
-        /* Sets the command to always run, even on `dry-run` mode. */
-        CommandOptionsBuilder& Always();
-        /* Sets the command's RootMode as `SU_ROOT` */
-        CommandOptionsBuilder& AsRoot();
-        /* Sets the command's RootMode as `DROP_ROOT` */
-        CommandOptionsBuilder& DropRoot();
-        /* Sets the command's StdoutMode `REDIRECT_TO_STDERR` */
-        CommandOptionsBuilder& RedirectStderr();
-        /* When not empty, logs a message before executing the command.
-         * Must contain a `%s`, which will be replaced by the full command line, and end on `\n`. */
-        CommandOptionsBuilder& Log(const std::string& message);
-        /* Builds the command options. */
-        CommandOptions Build();
-
-      private:
-        CommandOptionsBuilder(long timeout);
-        CommandOptionsValues values;
-        friend class CommandOptions;
-    };
-
-    /** Gets the command timeout, in seconds. */
-    long Timeout() const;
-    /* Checks whether the command should always be run, even on dry-run mode. */
-    bool Always() const;
-    /** Gets the RootMode of the command. */
-    RootMode RootMode() const;
-    /** Gets the StdoutMode of the command. */
-    StdoutMode StdoutMode() const;
-    /** Gets the logging message header, it any. */
-    std::string LoggingMessage() const;
-
-    /** Creates a builder with the requied timeout. */
-    static CommandOptionsBuilder WithTimeout(long timeout);
-
-    // Common options.
-    static CommandOptions DEFAULT;
-    static CommandOptions DEFAULT_DUMPSYS;
-    static CommandOptions AS_ROOT_5;
-    static CommandOptions AS_ROOT_10;
-    static CommandOptions AS_ROOT_20;
-};
-
-/*
  * Keeps track of current progress and estimated max, saving stats on file to tune up future runs.
  *
  * Each `dumpstate` section contributes to the total weight by an individual weight, so the overall
@@ -272,21 +152,12 @@
     friend class DumpstateTest;
 
   public:
+    static CommandOptions DEFAULT_DUMPSYS;
+
     static Dumpstate& GetInstance();
 
-    /*
-     * When running in dry-run mode, skips the real dumps and just print the section headers.
-     *
-     * Useful when debugging dumpstate or other bugreport-related activities.
-     *
-     * Dry-run mode is enabled by setting the system property dumpstate.dry_run to true.
-     */
-    bool IsDryRun() const;
-
-    /*
-     * Gets whether device is running a `user` build.
-     */
-    bool IsUserBuild() const;
+    // TODO: temporary function until device code uses PropertiesHelper::IsUserBuild()
+    bool IsUserBuild();
 
     /* Checkes whether dumpstate is generating a zipped bugreport. */
     bool IsZipping() const;
@@ -316,8 +187,7 @@
      * timeout from `options`)
      */
     void RunDumpsys(const std::string& title, const std::vector<std::string>& dumpsys_args,
-                    const CommandOptions& options = CommandOptions::DEFAULT_DUMPSYS,
-                    long dumpsys_timeout = 0);
+                    const CommandOptions& options = DEFAULT_DUMPSYS, long dumpsys_timeout = 0);
 
     /*
      * Prints the contents of a file.
@@ -362,6 +232,8 @@
     // TODO: temporary method until Dumpstate object is properly set
     void SetProgress(std::unique_ptr<Progress> progress);
 
+    void DumpstateBoard();
+
     /*
      * Updates the overall progress of the bugreport generation by the given weight increment.
      */
@@ -451,21 +323,7 @@
 
   private:
     // Used by GetInstance() only.
-    Dumpstate(const std::string& version = VERSION_CURRENT, bool dry_run = false,
-              const std::string& build_type = "user");
-
-    // Internal version of RunCommand that just runs it, without updating progress.
-    int JustRunCommand(const char* command, const char* path, std::vector<const char*>& args,
-                       const CommandOptions& options) const;
-
-    // Internal version of RunCommand that just dumps it, without updating progress.
-    int JustDumpFile(const std::string& title, const std::string& path) const;
-
-    // Whether this is a dry run.
-    bool dry_run_;
-
-    // Build type (such as 'user' or 'eng').
-    std::string build_type_;
+    Dumpstate(const std::string& version = VERSION_CURRENT);
 
     DISALLOW_COPY_AND_ASSIGN(Dumpstate);
 };
@@ -493,9 +351,6 @@
 int dump_files(const std::string& title, const char* dir, bool (*skip)(const char* path),
                int (*dump_from_fd)(const char* title, const char* path, int fd));
 
-/* switch to non-root user and group */
-bool drop_root_user();
-
 /* sends a broadcast using Activity Manager */
 void send_broadcast(const std::string& action, const std::vector<std::string>& args);
 
@@ -562,9 +417,6 @@
 /** Gets command-line arguments. */
 void format_args(int argc, const char *argv[], std::string *args);
 
-/** Tells if the device is running a user build. */
-bool is_user_build();
-
 #ifdef __cplusplus
 }
 #endif
diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp
index 0d68901..2e35112 100644
--- a/cmds/dumpstate/tests/dumpstate_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_test.cpp
@@ -17,6 +17,7 @@
 #define LOG_TAG "dumpstate"
 #include <cutils/log.h>
 
+#include "DumpstateInternal.h"
 #include "DumpstateService.h"
 #include "android/os/BnDumpstate.h"
 #include "dumpstate.h"
@@ -24,6 +25,7 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <fcntl.h>
 #include <libgen.h>
 #include <signal.h>
 #include <sys/types.h>
@@ -38,6 +40,7 @@
 using namespace android;
 
 using ::testing::EndsWith;
+using ::testing::HasSubstr;
 using ::testing::IsNull;
 using ::testing::IsEmpty;
 using ::testing::NotNull;
@@ -66,8 +69,34 @@
     MOCK_METHOD0(onAsBinder, IBinder*());
 };
 
+static int calls_;
+
 // Base class for all tests in this file
 class DumpstateBaseTest : public Test {
+  public:
+    virtual void SetUp() override {
+        calls_++;
+        SetDryRun(false);
+    }
+
+    void SetDryRun(bool dry_run) const {
+        PropertiesHelper::dry_run_ = dry_run;
+    }
+
+    void SetBuildType(const std::string& build_type) const {
+        PropertiesHelper::build_type_ = build_type;
+    }
+
+    bool IsStandalone() const {
+        return calls_ == 1;
+    }
+
+    void DropRoot() const {
+        DropRootUser();
+        uid_t uid = getuid();
+        ASSERT_EQ(2000, (int)uid);
+    }
+
   protected:
     const std::string kTestPath = dirname(android::base::GetExecutablePath().c_str());
     const std::string kFixturesPath = kTestPath + "/../dumpstate_test_fixture/";
@@ -91,20 +120,29 @@
         return to.c_str();
     }
 
-  private:
-    // Need a function that returns void to use assertions -
+    // Need functions that returns void to use assertions -
     // https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#assertion-placement
+    void ReadFileToString(const std::string& path, std::string* content) {
+        ASSERT_TRUE(android::base::ReadFileToString(path, content))
+            << "could not read contents from " << path;
+    }
+    void WriteStringToFile(const std::string& content, const std::string& path) {
+        ASSERT_TRUE(android::base::WriteStringToFile(content, path))
+            << "could not write contents to " << path;
+    }
+
+  private:
     void CopyTextFile(const std::string& from, const std::string& to) {
         std::string content;
-        ASSERT_TRUE(android::base::ReadFileToString(from, &content)) << "could not read from "
-                                                                     << from;
-        ASSERT_TRUE(android::base::WriteStringToFile(content, to)) << "could not write to " << to;
+        ReadFileToString(from, &content);
+        WriteStringToFile(content, to);
     }
 };
 
 class DumpstateTest : public DumpstateBaseTest {
   public:
     void SetUp() {
+        DumpstateBaseTest::SetUp();
         SetDryRun(false);
         SetBuildType(android::base::GetProperty("ro.build.type", "(unknown)"));
         ds.progress_.reset(new Progress());
@@ -133,26 +171,6 @@
         return status;
     }
 
-    void SetDryRun(bool dry_run) {
-        ALOGD("Setting dry_run_ to %s\n", dry_run ? "true" : "false");
-        ds.dry_run_ = dry_run;
-    }
-
-    void SetBuildType(const std::string& build_type) {
-        ALOGD("Setting build_type_ to '%s'\n", build_type.c_str());
-        ds.build_type_ = build_type;
-    }
-
-    bool IsUserBuild() {
-        return "user" == android::base::GetProperty("ro.build.type", "(unknown)");
-    }
-
-    void DropRoot() {
-        drop_root_user();
-        uid_t uid = getuid();
-        ASSERT_EQ(2000, (int)uid);
-    }
-
     void SetProgress(long progress, long initial_max, long threshold = 0) {
         ds.update_progress_ = true;
         ds.update_progress_threshold_ = threshold;
@@ -401,6 +419,12 @@
 }
 
 TEST_F(DumpstateTest, RunCommandDropRoot) {
+    if (!IsStandalone()) {
+        // TODO: temporarily disabled because it might cause other tests to fail after dropping
+        // to Shell - need to refactor tests to avoid this problem)
+        MYLOGE("Skipping DumpstateTest.RunCommandDropRoot() on test suite\n")
+        return;
+    }
     // First check root case - only available when running with 'adb root'.
     uid_t uid = getuid();
     if (uid == 0) {
@@ -417,7 +441,13 @@
 }
 
 TEST_F(DumpstateTest, RunCommandAsRootUserBuild) {
-    if (!IsUserBuild()) {
+    if (!IsStandalone()) {
+        // TODO: temporarily disabled because it might cause other tests to fail after dropping
+        // to Shell - need to refactor tests to avoid this problem)
+        MYLOGE("Skipping DumpstateTest.RunCommandAsRootUserBuild() on test suite\n")
+        return;
+    }
+    if (!PropertiesHelper::IsUserBuild()) {
         // Emulates user build if necessarily.
         SetBuildType("user");
     }
@@ -432,6 +462,27 @@
     EXPECT_THAT(err, IsEmpty());
 }
 
+TEST_F(DumpstateTest, RunCommandAsRootNonUserBuild) {
+    if (!IsStandalone()) {
+        // TODO: temporarily disabled because it might cause other tests to fail after dropping
+        // to Shell - need to refactor tests to avoid this problem)
+        MYLOGE("Skipping DumpstateTest.RunCommandAsRootNonUserBuild() on test suite\n")
+        return;
+    }
+    if (PropertiesHelper::IsUserBuild()) {
+        ALOGI("Skipping RunCommandAsRootNonUserBuild on user builds\n");
+        return;
+    }
+
+    DropRoot();
+
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand, "--uid"},
+                            CommandOptions::WithTimeout(1).AsRoot().Build()));
+
+    EXPECT_THAT(out, StrEq("0\nstdout\n"));
+    EXPECT_THAT(err, StrEq("stderr\n"));
+}
+
 TEST_F(DumpstateTest, DumpFileNotFoundNoTitle) {
     EXPECT_EQ(-1, DumpFile("", "/I/cant/believe/I/exist"));
     EXPECT_THAT(out,
@@ -483,10 +534,10 @@
     SetDryRun(true);
     EXPECT_EQ(0, DumpFile("Might as well dump. Dump!", kTestDataPath + "single-line.txt"));
     EXPECT_THAT(err, IsEmpty());
-    EXPECT_THAT(out, StartsWith("------ Might as well dump. Dump! (" + kTestDataPath +
-                                "single-line.txt) ------\n\t(skipped on dry run)\n------"));
+    EXPECT_THAT(
+        out, StartsWith("------ Might as well dump. Dump! (" + kTestDataPath + "single-line.txt:"));
+    EXPECT_THAT(out, HasSubstr("\n\t(skipped on dry run)\n------"));
     EXPECT_THAT(out, EndsWith("s was the duration of 'Might as well dump. Dump!' ------\n"));
-    EXPECT_THAT(err, IsEmpty());
 }
 
 TEST_F(DumpstateTest, DumpFileUpdateProgress) {
@@ -547,8 +598,7 @@
         std::string expected_content =
             android::base::StringPrintf("%d %d\n", expected_runs, expected_average);
         std::string actual_content;
-        ASSERT_TRUE(android::base::ReadFileToString(path, &actual_content))
-            << "could not read statsfrom" << path;
+        ReadFileToString(path, &actual_content);
         ASSERT_THAT(actual_content, StrEq(expected_content)) << "invalid stats on " << path;
     }
 };
@@ -658,6 +708,12 @@
 
 // Tests stats are properly saved when the file does not exists.
 TEST_F(ProgressTest, FirstTime) {
+    if (!IsStandalone()) {
+        // TODO: temporarily disabled because it's failing when running as suite
+        MYLOGE("Skipping ProgressTest.FirstTime() on test suite\n")
+        return;
+    }
+
     std::string path = kTestDataPath + "FirstTime.txt";
     android::base::RemoveFileIfExists(path);
 
@@ -745,11 +801,243 @@
     AssertStats(path, 3, 16);
 }
 
-// TODO: RunCommandAsRootNonUserBuild must be the last test because it drops root, which could cause
-// other tests to fail if they're relyin on the process running as root.
-// For now this is fine, but eventually it might need to be moved to its own test case / process.
-TEST_F(DumpstateTest, RunCommandAsRootNonUserBuild) {
-    if (IsUserBuild()) {
+class DumpstateUtilTest : public DumpstateBaseTest {
+  public:
+    void SetUp() {
+        DumpstateBaseTest::SetUp();
+        SetDryRun(false);
+    }
+
+    void CaptureFdOut() {
+        ReadFileToString(path_, &out);
+    }
+
+    void CreateFd(const std::string& name) {
+        path_ = kTestDataPath + name;
+        MYLOGD("Creating fd for file %s\n", path_.c_str());
+
+        fd = TEMP_FAILURE_RETRY(open(path_.c_str(),
+                                     O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW,
+                                     S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
+        ASSERT_GE(fd, 0) << "could not create FD for path " << path_;
+    }
+
+    // Runs a command into the `fd` and capture `stderr`.
+    int RunCommand(const std::string& title, const std::vector<std::string>& full_command,
+                   const CommandOptions& options = CommandOptions::DEFAULT) {
+        CaptureStderr();
+        int status = RunCommandToFd(fd, title, full_command, options);
+        close(fd);
+
+        CaptureFdOut();
+        err = GetCapturedStderr();
+        return status;
+    }
+
+    // Dumps a file and into the `fd` and `stderr`.
+    int DumpFile(const std::string& title, const std::string& path) {
+        CaptureStderr();
+        int status = DumpFileToFd(fd, title, path);
+        close(fd);
+
+        CaptureFdOut();
+        err = GetCapturedStderr();
+        return status;
+    }
+
+    int fd;
+
+    // 'fd` output and `stderr` from the last command ran.
+    std::string out, err;
+
+  private:
+    std::string path_;
+};
+
+TEST_F(DumpstateUtilTest, RunCommandNoArgs) {
+    CreateFd("RunCommandNoArgs.txt");
+    EXPECT_EQ(-1, RunCommand("", {}));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandNoTitle) {
+    CreateFd("RunCommandWithNoArgs.txt");
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}));
+    EXPECT_THAT(out, StrEq("stdout\n"));
+    EXPECT_THAT(err, StrEq("stderr\n"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandWithTitle) {
+    CreateFd("RunCommandWithNoArgs.txt");
+    EXPECT_EQ(0, RunCommand("I AM GROOT", {kSimpleCommand}));
+    EXPECT_THAT(out, StrEq("------ I AM GROOT (" + kSimpleCommand + ") ------\nstdout\n"));
+    EXPECT_THAT(err, StrEq("stderr\n"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandWithOneArg) {
+    CreateFd("RunCommandWithOneArg.txt");
+    EXPECT_EQ(0, RunCommand("", {kEchoCommand, "one"}));
+    EXPECT_THAT(err, IsEmpty());
+    EXPECT_THAT(out, StrEq("one\n"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandWithMultipleArgs) {
+    CreateFd("RunCommandWithMultipleArgs.txt");
+    EXPECT_EQ(0, RunCommand("", {kEchoCommand, "one", "is", "the", "loniest", "number"}));
+    EXPECT_THAT(err, IsEmpty());
+    EXPECT_THAT(out, StrEq("one is the loniest number\n"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandWithLoggingMessage) {
+    CreateFd("RunCommandWithLoggingMessage.txt");
+    EXPECT_EQ(
+        0, RunCommand("", {kSimpleCommand},
+                      CommandOptions::WithTimeout(10).Log("COMMAND, Y U NO LOG FIRST?").Build()));
+    EXPECT_THAT(out, StrEq("stdout\n"));
+    EXPECT_THAT(err, StrEq("COMMAND, Y U NO LOG FIRST?stderr\n"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandRedirectStderr) {
+    CreateFd("RunCommandRedirectStderr.txt");
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand},
+                            CommandOptions::WithTimeout(10).RedirectStderr().Build()));
+    EXPECT_THAT(out, IsEmpty());
+    EXPECT_THAT(err, StrEq("stdout\nstderr\n"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandDryRun) {
+    CreateFd("RunCommandDryRun.txt");
+    SetDryRun(true);
+    EXPECT_EQ(0, RunCommand("I AM GROOT", {kSimpleCommand}));
+    EXPECT_THAT(out, StrEq(android::base::StringPrintf(
+                         "------ I AM GROOT (%s) ------\n\t(skipped on dry run)\n",
+                         kSimpleCommand.c_str())));
+    EXPECT_THAT(err, IsEmpty());
+}
+
+TEST_F(DumpstateUtilTest, RunCommandDryRunNoTitle) {
+    CreateFd("RunCommandDryRun.txt");
+    SetDryRun(true);
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}));
+    EXPECT_THAT(
+        out, StrEq(android::base::StringPrintf("%s: skipped on dry run\n", kSimpleCommand.c_str())));
+    EXPECT_THAT(err, IsEmpty());
+}
+
+TEST_F(DumpstateUtilTest, RunCommandDryRunAlways) {
+    CreateFd("RunCommandDryRunAlways.txt");
+    SetDryRun(true);
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(10).Always().Build()));
+    EXPECT_THAT(out, StrEq("stdout\n"));
+    EXPECT_THAT(err, StrEq("stderr\n"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandNotFound) {
+    CreateFd("RunCommandNotFound.txt");
+    EXPECT_NE(0, RunCommand("", {"/there/cannot/be/such/command"}));
+    EXPECT_THAT(out, StartsWith("*** command '/there/cannot/be/such/command' failed: exit code"));
+    EXPECT_THAT(err, StartsWith("execvp on command '/there/cannot/be/such/command' failed"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandFails) {
+    CreateFd("RunCommandFails.txt");
+    EXPECT_EQ(42, RunCommand("", {kSimpleCommand, "--exit", "42"}));
+    EXPECT_THAT(out, StrEq("stdout\n*** command '" + kSimpleCommand +
+                           " --exit 42' failed: exit code 42\n"));
+    EXPECT_THAT(err, StrEq("stderr\n*** command '" + kSimpleCommand +
+                           " --exit 42' failed: exit code 42\n"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandCrashes) {
+    CreateFd("RunCommandCrashes.txt");
+    EXPECT_NE(0, RunCommand("", {kSimpleCommand, "--crash"}));
+    // We don't know the exit code, so check just the prefix.
+    EXPECT_THAT(
+        out, StartsWith("stdout\n*** command '" + kSimpleCommand + " --crash' failed: exit code"));
+    EXPECT_THAT(
+        err, StartsWith("stderr\n*** command '" + kSimpleCommand + " --crash' failed: exit code"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandTimesout) {
+    CreateFd("RunCommandTimesout.txt");
+    EXPECT_EQ(-1, RunCommand("", {kSimpleCommand, "--sleep", "2"},
+                             CommandOptions::WithTimeout(1).Build()));
+    EXPECT_THAT(out, StartsWith("stdout line1\n*** command '" + kSimpleCommand +
+                                " --sleep 2' timed out after 1"));
+    EXPECT_THAT(err, StartsWith("sleeping for 2s\n*** command '" + kSimpleCommand +
+                                " --sleep 2' timed out after 1"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandIsKilled) {
+    CreateFd("RunCommandIsKilled.txt");
+    CaptureStderr();
+
+    std::thread t([=]() {
+        EXPECT_EQ(SIGTERM, RunCommandToFd(fd, "", {kSimpleCommand, "--pid", "--sleep", "20"},
+                                          CommandOptions::WithTimeout(100).Always().Build()));
+    });
+
+    // Capture pid and pre-sleep output.
+    sleep(1);  // Wait a little bit to make sure pid and 1st line were printed.
+    std::string err = GetCapturedStderr();
+    EXPECT_THAT(err, StrEq("sleeping for 20s\n"));
+
+    CaptureFdOut();
+    std::vector<std::string> lines = android::base::Split(out, "\n");
+    ASSERT_EQ(3, (int)lines.size()) << "Invalid lines before sleep: " << out;
+
+    int pid = atoi(lines[0].c_str());
+    EXPECT_THAT(lines[1], StrEq("stdout line1"));
+    EXPECT_THAT(lines[2], IsEmpty());  // \n
+
+    // Then kill the process.
+    CaptureFdOut();
+    CaptureStderr();
+    ASSERT_EQ(0, kill(pid, SIGTERM)) << "failed to kill pid " << pid;
+    t.join();
+
+    // Finally, check output after murder.
+    CaptureFdOut();
+    err = GetCapturedStderr();
+
+    // out starts with the pid, which is an unknown
+    EXPECT_THAT(out, EndsWith("stdout line1\n*** command '" + kSimpleCommand +
+                              " --pid --sleep 20' failed: killed by signal 15\n"));
+    EXPECT_THAT(err, StrEq("*** command '" + kSimpleCommand +
+                           " --pid --sleep 20' failed: killed by signal 15\n"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandAsRootUserBuild) {
+    if (!IsStandalone()) {
+        // TODO: temporarily disabled because it might cause other tests to fail after dropping
+        // to Shell - need to refactor tests to avoid this problem)
+        MYLOGE("Skipping DumpstateUtilTest.RunCommandAsRootUserBuild() on test suite\n")
+        return;
+    }
+    CreateFd("RunCommandAsRootUserBuild.txt");
+    if (!PropertiesHelper::IsUserBuild()) {
+        // Emulates user build if necessarily.
+        SetBuildType("user");
+    }
+
+    DropRoot();
+
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(1).AsRoot().Build()));
+
+    // We don't know the exact path of su, so we just check for the 'root ...' commands
+    EXPECT_THAT(out, StartsWith("Skipping"));
+    EXPECT_THAT(out, EndsWith("root " + kSimpleCommand + "' on user build.\n"));
+    EXPECT_THAT(err, IsEmpty());
+}
+
+TEST_F(DumpstateUtilTest, RunCommandAsRootNonUserBuild) {
+    if (!IsStandalone()) {
+        // TODO: temporarily disabled because it might cause other tests to fail after dropping
+        // to Shell - need to refactor tests to avoid this problem)
+        MYLOGE("Skipping DumpstateUtilTest.RunCommandAsRootNonUserBuild() on test suite\n")
+        return;
+    }
+    CreateFd("RunCommandAsRootNonUserBuild.txt");
+    if (PropertiesHelper::IsUserBuild()) {
         ALOGI("Skipping RunCommandAsRootNonUserBuild on user builds\n");
         return;
     }
@@ -762,3 +1050,90 @@
     EXPECT_THAT(out, StrEq("0\nstdout\n"));
     EXPECT_THAT(err, StrEq("stderr\n"));
 }
+
+TEST_F(DumpstateUtilTest, RunCommandDropRoot) {
+    if (!IsStandalone()) {
+        // TODO: temporarily disabled because it might cause other tests to fail after dropping
+        // to Shell - need to refactor tests to avoid this problem)
+        MYLOGE("Skipping DumpstateUtilTest.RunCommandDropRoot() on test suite\n")
+        return;
+    }
+    CreateFd("RunCommandDropRoot.txt");
+    // First check root case - only available when running with 'adb root'.
+    uid_t uid = getuid();
+    if (uid == 0) {
+        EXPECT_EQ(0, RunCommand("", {kSimpleCommand, "--uid"}));
+        EXPECT_THAT(out, StrEq("0\nstdout\n"));
+        EXPECT_THAT(err, StrEq("stderr\n"));
+        return;
+    }
+    // Then run dropping root.
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand, "--uid"},
+                            CommandOptions::WithTimeout(1).DropRoot().Build()));
+    EXPECT_THAT(out, StrEq("2000\nstdout\n"));
+    EXPECT_THAT(err, StrEq("drop_root_user(): already running as Shell\nstderr\n"));
+}
+
+TEST_F(DumpstateUtilTest, DumpFileNotFoundNoTitle) {
+    CreateFd("DumpFileNotFound.txt");
+    EXPECT_EQ(-1, DumpFile("", "/I/cant/believe/I/exist"));
+    EXPECT_THAT(out,
+                StrEq("*** Error dumping /I/cant/believe/I/exist: No such file or directory\n"));
+    EXPECT_THAT(err, IsEmpty());
+}
+
+TEST_F(DumpstateUtilTest, DumpFileNotFoundWithTitle) {
+    CreateFd("DumpFileNotFound.txt");
+    EXPECT_EQ(-1, DumpFile("Y U NO EXIST?", "/I/cant/believe/I/exist"));
+    EXPECT_THAT(out, StrEq("*** Error dumping /I/cant/believe/I/exist (Y U NO EXIST?): No such "
+                           "file or directory\n"));
+    EXPECT_THAT(err, IsEmpty());
+}
+
+TEST_F(DumpstateUtilTest, DumpFileSingleLine) {
+    CreateFd("DumpFileSingleLine.txt");
+    EXPECT_EQ(0, DumpFile("", kTestDataPath + "single-line.txt"));
+    EXPECT_THAT(err, IsEmpty());
+    EXPECT_THAT(out, StrEq("I AM LINE1\n"));  // dumpstate adds missing newline
+}
+
+TEST_F(DumpstateUtilTest, DumpFileSingleLineWithNewLine) {
+    CreateFd("DumpFileSingleLineWithNewLine.txt");
+    EXPECT_EQ(0, DumpFile("", kTestDataPath + "single-line-with-newline.txt"));
+    EXPECT_THAT(err, IsEmpty());
+    EXPECT_THAT(out, StrEq("I AM LINE1\n"));
+}
+
+TEST_F(DumpstateUtilTest, DumpFileMultipleLines) {
+    CreateFd("DumpFileMultipleLines.txt");
+    EXPECT_EQ(0, DumpFile("", kTestDataPath + "multiple-lines.txt"));
+    EXPECT_THAT(err, IsEmpty());
+    EXPECT_THAT(out, StrEq("I AM LINE1\nI AM LINE2\nI AM LINE3\n"));
+}
+
+TEST_F(DumpstateUtilTest, DumpFileMultipleLinesWithNewLine) {
+    CreateFd("DumpFileMultipleLinesWithNewLine.txt");
+    EXPECT_EQ(0, DumpFile("", kTestDataPath + "multiple-lines-with-newline.txt"));
+    EXPECT_THAT(err, IsEmpty());
+    EXPECT_THAT(out, StrEq("I AM LINE1\nI AM LINE2\nI AM LINE3\n"));
+}
+
+TEST_F(DumpstateUtilTest, DumpFileOnDryRunNoTitle) {
+    CreateFd("DumpFileOnDryRun.txt");
+    SetDryRun(true);
+    std::string path = kTestDataPath + "single-line.txt";
+    EXPECT_EQ(0, DumpFile("", kTestDataPath + "single-line.txt"));
+    EXPECT_THAT(err, IsEmpty());
+    EXPECT_THAT(out, StrEq(path + ": skipped on dry run\n"));
+}
+
+TEST_F(DumpstateUtilTest, DumpFileOnDryRun) {
+    CreateFd("DumpFileOnDryRun.txt");
+    SetDryRun(true);
+    std::string path = kTestDataPath + "single-line.txt";
+    EXPECT_EQ(0, DumpFile("Might as well dump. Dump!", kTestDataPath + "single-line.txt"));
+    EXPECT_THAT(err, IsEmpty());
+    EXPECT_THAT(
+        out, StartsWith("------ Might as well dump. Dump! (" + kTestDataPath + "single-line.txt:"));
+    EXPECT_THAT(out, EndsWith("skipped on dry run\n"));
+}
diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp
index b5f328d..01a60da 100644
--- a/cmds/dumpstate/utils.cpp
+++ b/cmds/dumpstate/utils.cpp
@@ -14,63 +14,42 @@
  * limitations under the License.
  */
 
+#define LOG_TAG "dumpstate"
+
+#include "dumpstate.h"
+
 #include <dirent.h>
-#include <errno.h>
 #include <fcntl.h>
 #include <libgen.h>
-#include <limits.h>
 #include <math.h>
 #include <poll.h>
-#include <signal.h>
-#include <stdarg.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <sys/capability.h>
 #include <sys/inotify.h>
 #include <sys/klog.h>
-#include <sys/prctl.h>
-#include <sys/stat.h>
-#include <sys/time.h>
-#include <sys/wait.h>
-#include <time.h>
-#include <unistd.h>
-#include <string>
-#include <vector>
-
-#define LOG_TAG "dumpstate"
 
 #include <android-base/file.h>
 #include <android-base/properties.h>
 #include <android-base/stringprintf.h>
 #include <android-base/strings.h>
 #include <cutils/debugger.h>
-#include <cutils/log.h>
 #include <cutils/properties.h>
 #include <cutils/sockets.h>
 #include <private/android_filesystem_config.h>
 
-#include <selinux/android.h>
-
-#include "dumpstate.h"
-
-#define SU_PATH "/system/xbin/su"
-
-static const int64_t NANOS_PER_SEC = 1000000000;
+#include "DumpstateInternal.h"
 
 static const int TRACE_DUMP_TIMEOUT_MS = 10000; // 10 seconds
 
+/* Most simple commands have 10 as timeout, so 5 is a good estimate */
+static const int32_t WEIGHT_FILE = 5;
+
 // TODO: temporary variables and functions used during C++ refactoring
 static Dumpstate& ds = Dumpstate::GetInstance();
 static int RunCommand(const std::string& title, const std::vector<std::string>& full_command,
                       const CommandOptions& options = CommandOptions::DEFAULT) {
     return ds.RunCommand(title, full_command, options);
 }
-static bool IsDryRun() {
-    return Dumpstate::GetInstance().IsDryRun();
-}
-static void UpdateProgress(int32_t delta) {
-    ds.UpdateProgress(delta);
+bool Dumpstate::IsUserBuild() {
+    return PropertiesHelper::IsUserBuild();
 }
 
 /* list of native processes to include in the native dumps */
@@ -89,130 +68,42 @@
         NULL,
 };
 
-/* Most simple commands have 10 as timeout, so 5 is a good estimate */
-static const int WEIGHT_FILE = 5;
-
 // Reasonable value for max stats.
 static const int STATS_MAX_N_RUNS = 1000;
 static const long STATS_MAX_AVERAGE = 100000;
 
-CommandOptions CommandOptions::DEFAULT = CommandOptions::WithTimeout(10).Build();
-CommandOptions CommandOptions::DEFAULT_DUMPSYS = CommandOptions::WithTimeout(30).Build();
-CommandOptions CommandOptions::AS_ROOT_5 = CommandOptions::WithTimeout(5).AsRoot().Build();
-CommandOptions CommandOptions::AS_ROOT_10 = CommandOptions::WithTimeout(10).AsRoot().Build();
-CommandOptions CommandOptions::AS_ROOT_20 = CommandOptions::WithTimeout(20).AsRoot().Build();
+CommandOptions Dumpstate::DEFAULT_DUMPSYS = CommandOptions::WithTimeout(30).Build();
 
-CommandOptions::CommandOptionsBuilder::CommandOptionsBuilder(long timeout) : values(timeout) {
-}
-
-CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::Always() {
-    values.always_ = true;
-    return *this;
-}
-
-CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::AsRoot() {
-    values.root_mode_ = SU_ROOT;
-    return *this;
-}
-
-CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::DropRoot() {
-    values.root_mode_ = DROP_ROOT;
-    return *this;
-}
-
-CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::RedirectStderr() {
-    values.stdout_mode_ = REDIRECT_TO_STDERR;
-    return *this;
-}
-
-CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::Log(
-    const std::string& message) {
-    values.logging_message_ = message;
-    return *this;
-}
-
-CommandOptions CommandOptions::CommandOptionsBuilder::Build() {
-    return CommandOptions(values);
-}
-
-CommandOptions::CommandOptionsValues::CommandOptionsValues(long timeout)
-    : timeout_(timeout),
-      always_(false),
-      root_mode_(DONT_DROP_ROOT),
-      stdout_mode_(NORMAL_STDOUT),
-      logging_message_("") {
-}
-
-CommandOptions::CommandOptions(const CommandOptionsValues& values) : values(values) {
-}
-
-long CommandOptions::Timeout() const {
-    return values.timeout_;
-}
-
-bool CommandOptions::Always() const {
-    return values.always_;
-}
-
-RootMode CommandOptions::RootMode() const {
-    return values.root_mode_;
-}
-
-StdoutMode CommandOptions::StdoutMode() const {
-    return values.stdout_mode_;
-}
-
-std::string CommandOptions::LoggingMessage() const {
-    return values.logging_message_;
-}
-
-CommandOptions::CommandOptionsBuilder CommandOptions::WithTimeout(long timeout) {
-    return CommandOptions::CommandOptionsBuilder(timeout);
-}
-
-Dumpstate::Dumpstate(const std::string& version, bool dry_run, const std::string& build_type)
-    : pid_(getpid()),
-      version_(version),
-      now_(time(nullptr)),
-      dry_run_(dry_run),
-      build_type_(build_type) {
+Dumpstate::Dumpstate(const std::string& version)
+    : pid_(getpid()), version_(version), now_(time(nullptr)) {
 }
 
 Dumpstate& Dumpstate::GetInstance() {
-    static Dumpstate singleton_(android::base::GetProperty("dumpstate.version", VERSION_CURRENT),
-                                android::base::GetBoolProperty("dumpstate.dry_run", false),
-                                android::base::GetProperty("ro.build.type", "(unknown)"));
+    static Dumpstate singleton_(android::base::GetProperty("dumpstate.version", VERSION_CURRENT));
     return singleton_;
 }
 
-DurationReporter::DurationReporter(const std::string& title) : DurationReporter(title, stdout) {
-}
-
-DurationReporter::DurationReporter(const std::string& title, FILE* out) : title_(title), out_(out) {
+DurationReporter::DurationReporter(const std::string& title, bool log_only)
+    : title_(title), log_only_(log_only) {
     if (!title_.empty()) {
-        started_ = DurationReporter::Nanotime();
+        started_ = Nanotime();
     }
 }
 
 DurationReporter::~DurationReporter() {
     if (!title_.empty()) {
-        uint64_t elapsed = DurationReporter::Nanotime() - started_;
-        // Use "Yoda grammar" to make it easier to grep|sort sections.
-        if (out_ != nullptr) {
-            fprintf(out_, "------ %.3fs was the duration of '%s' ------\n",
-                    (float)elapsed / NANOS_PER_SEC, title_.c_str());
-        } else {
+        uint64_t elapsed = Nanotime() - started_;
+        if (log_only_) {
             MYLOGD("Duration of '%s': %.3fs\n", title_.c_str(), (float)elapsed / NANOS_PER_SEC);
+        } else {
+            // Use "Yoda grammar" to make it easier to grep|sort sections.
+            printf("------ %.3fs was the duration of '%s' ------\n", (float)elapsed / NANOS_PER_SEC,
+                   title_.c_str());
+            fflush(stdout);
         }
     }
 }
 
-uint64_t DurationReporter::DurationReporter::Nanotime() {
-    struct timespec ts;
-    clock_gettime(CLOCK_MONOTONIC, &ts);
-    return (uint64_t) ts.tv_sec * NANOS_PER_SEC + ts.tv_nsec;
-}
-
 const int32_t Progress::kDefaultMax = 5000;
 
 Progress::Progress(const std::string& path) : Progress(Progress::kDefaultMax, 1.1, path) {
@@ -322,14 +213,6 @@
     dprintf(fd, "%saverage_max: %d\n", pr, average_max_);
 }
 
-bool Dumpstate::IsDryRun() const {
-    return dry_run_;
-}
-
-bool Dumpstate::IsUserBuild() const {
-    return "user" == build_type_;
-}
-
 bool Dumpstate::IsZipping() const {
     return zip_writer_ != nullptr;
 }
@@ -344,7 +227,7 @@
 }
 
 void for_each_userid(void (*func)(int), const char *header) {
-    if (IsDryRun()) return;
+    if (PropertiesHelper::IsDryRun()) return;
 
     DIR *d;
     struct dirent *de;
@@ -427,7 +310,7 @@
 }
 
 void for_each_pid(for_each_pid_func func, const char *header) {
-    if (IsDryRun()) return;
+    if (PropertiesHelper::IsDryRun()) return;
 
     __for_each_pid(for_each_pid_helper, header, (void *) func);
 }
@@ -481,13 +364,13 @@
 }
 
 void for_each_tid(for_each_tid_func func, const char *header) {
-    if (IsDryRun()) return;
+    if (PropertiesHelper::IsDryRun()) return;
 
     __for_each_pid(for_each_tid_helper, header, (void *) func);
 }
 
 void show_wchan(int pid, int tid, const char *name) {
-    if (IsDryRun()) return;
+    if (PropertiesHelper::IsDryRun()) return;
 
     char path[255];
     char buffer[255];
@@ -554,7 +437,7 @@
 }
 
 void show_showtime(int pid, const char *name) {
-    if (IsDryRun()) return;
+    if (PropertiesHelper::IsDryRun()) return;
 
     char path[255];
     char buffer[1023];
@@ -622,7 +505,7 @@
     DurationReporter duration_reporter(title);
     printf("------ %s ------\n", title);
 
-    if (IsDryRun()) return;
+    if (PropertiesHelper::IsDryRun()) return;
 
     /* Get size of kernel buffer */
     int size = klogctl(KLOG_SIZE_BUFFER, NULL, 0);
@@ -653,98 +536,19 @@
 
     snprintf(title, sizeof(title), "SHOW MAP %d (%s)", pid, name);
     snprintf(arg, sizeof(arg), "%d", pid);
-    RunCommand(title, {"showmap", "-q", arg}, CommandOptions::AS_ROOT_10);
-}
-
-// TODO: when converted to a Dumpstate function, it should be const
-static int _dump_file_from_fd(const std::string& title, const char* path, int fd) {
-    if (!title.empty()) {
-        printf("------ %s (%s", title.c_str(), path);
-
-        struct stat st;
-        // Only show the modification time of non-device files.
-        size_t path_len = strlen(path);
-        if ((path_len < 6 || memcmp(path, "/proc/", 6)) &&
-                (path_len < 5 || memcmp(path, "/sys/", 5)) &&
-                (path_len < 3 || memcmp(path, "/d/", 3)) &&
-                !fstat(fd, &st)) {
-            char stamp[80];
-            time_t mtime = st.st_mtime;
-            strftime(stamp, sizeof(stamp), "%Y-%m-%d %H:%M:%S", localtime(&mtime));
-            printf(": %s", stamp);
-        }
-        printf(") ------\n");
-    }
-
-    bool newline = false;
-    fd_set read_set;
-    struct timeval tm;
-    while (1) {
-        FD_ZERO(&read_set);
-        FD_SET(fd, &read_set);
-        /* Timeout if no data is read for 30 seconds. */
-        tm.tv_sec = 30;
-        tm.tv_usec = 0;
-        uint64_t elapsed = DurationReporter::Nanotime();
-        int ret = TEMP_FAILURE_RETRY(select(fd + 1, &read_set, NULL, NULL, &tm));
-        if (ret == -1) {
-            printf("*** %s: select failed: %s\n", path, strerror(errno));
-            newline = true;
-            break;
-        } else if (ret == 0) {
-            elapsed = DurationReporter::Nanotime() - elapsed;
-            printf("*** %s: Timed out after %.3fs\n", path,
-                   (float) elapsed / NANOS_PER_SEC);
-            newline = true;
-            break;
-        } else {
-            char buffer[65536];
-            ssize_t bytes_read = TEMP_FAILURE_RETRY(read(fd, buffer, sizeof(buffer)));
-            if (bytes_read > 0) {
-                fwrite(buffer, bytes_read, 1, stdout);
-                newline = (buffer[bytes_read-1] == '\n');
-            } else {
-                if (bytes_read == -1) {
-                    printf("*** %s: Failed to read from fd: %s", path, strerror(errno));
-                    newline = true;
-                }
-                break;
-            }
-        }
-    }
-    UpdateProgress(WEIGHT_FILE);
-    close(fd);
-
-    if (!newline) printf("\n");
-    if (!title.empty()) printf("\n");
-    return 0;
+    RunCommand(title, {"showmap", "-q", arg}, CommandOptions::AS_ROOT);
 }
 
 int Dumpstate::DumpFile(const std::string& title, const std::string& path) {
     DurationReporter duration_reporter(title);
-    if (IsDryRun()) {
-        if (!title.empty()) {
-            printf("------ %s (%s) ------\n", title.c_str(), path.c_str());
-            printf("\t(skipped on dry run)\n");
-        }
-        UpdateProgress(WEIGHT_FILE);
-        return 0;
-    }
-    return JustDumpFile(title, path);
-}
 
-int Dumpstate::JustDumpFile(const std::string& title, const std::string& path) const {
-    int fd = TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY | O_NONBLOCK | O_CLOEXEC));
-    if (fd < 0) {
-        int err = errno;
-        if (title.empty()) {
-            printf("*** Error dumping %s: %s\n", path.c_str(), strerror(err));
-        } else {
-            printf("*** Error dumping %s (%s): %s\n", path.c_str(), title.c_str(), strerror(err));
-        }
-        return -1;
-    }
-    return _dump_file_from_fd(title, path.c_str(), fd);
+    int status = DumpFileToFd(STDOUT_FILENO, title, path);
+
+    UpdateProgress(WEIGHT_FILE);
+
+    fflush(stdout);
+
+    return status;
 }
 
 int read_file_as_long(const char *path, long int *output) {
@@ -786,7 +590,7 @@
     if (!title.empty()) {
         printf("------ %s (%s) ------\n", title.c_str(), dir);
     }
-    if (IsDryRun()) return 0;
+    if (PropertiesHelper::IsDryRun()) return 0;
 
     if (dir[strlen(dir) - 1] == '/') {
         ++slash;
@@ -843,7 +647,7 @@
  * stuck.
  */
 int dump_file_from_fd(const char *title, const char *path, int fd) {
-    if (IsDryRun()) return 0;
+    if (PropertiesHelper::IsDryRun()) return 0;
 
     int flags = fcntl(fd, F_GETFL);
     if (flags == -1) {
@@ -855,216 +659,22 @@
         close(fd);
         return -1;
     }
-    return _dump_file_from_fd(title, path, fd);
-}
-
-bool waitpid_with_timeout(pid_t pid, int timeout_seconds, int* status) {
-    sigset_t child_mask, old_mask;
-    sigemptyset(&child_mask);
-    sigaddset(&child_mask, SIGCHLD);
-
-    if (sigprocmask(SIG_BLOCK, &child_mask, &old_mask) == -1) {
-        printf("*** sigprocmask failed: %s\n", strerror(errno));
-        return false;
-    }
-
-    struct timespec ts;
-    ts.tv_sec = timeout_seconds;
-    ts.tv_nsec = 0;
-    int ret = TEMP_FAILURE_RETRY(sigtimedwait(&child_mask, NULL, &ts));
-    int saved_errno = errno;
-    // Set the signals back the way they were.
-    if (sigprocmask(SIG_SETMASK, &old_mask, NULL) == -1) {
-        printf("*** sigprocmask failed: %s\n", strerror(errno));
-        if (ret == 0) {
-            return false;
-        }
-    }
-    if (ret == -1) {
-        errno = saved_errno;
-        if (errno == EAGAIN) {
-            errno = ETIMEDOUT;
-        } else {
-            printf("*** sigtimedwait failed: %s\n", strerror(errno));
-        }
-        return false;
-    }
-
-    pid_t child_pid = waitpid(pid, status, WNOHANG);
-    if (child_pid != pid) {
-        if (child_pid != -1) {
-            printf("*** Waiting for pid %d, got pid %d instead\n", pid, child_pid);
-        } else {
-            printf("*** waitpid failed: %s\n", strerror(errno));
-        }
-        return false;
-    }
-    return true;
+    return DumpFileFromFdToFd(title, path, fd, STDOUT_FILENO, PropertiesHelper::IsDryRun());
 }
 
 int Dumpstate::RunCommand(const std::string& title, const std::vector<std::string>& full_command,
                           const CommandOptions& options) {
-    if (full_command.empty()) {
-        MYLOGE("No arguments on command '%s'\n", title.c_str());
-        return -1;
-    }
-
-    int size = full_command.size() + 1;  // null terminated
-    int starting_index = 0;
-    if (options.RootMode() == SU_ROOT) {
-        starting_index = 2;  // "su" "root"
-        size += starting_index;
-    }
-
-    std::vector<const char*> args;
-    args.resize(size);
-
-    std::string command_string;
-    if (options.RootMode() == SU_ROOT) {
-        args[0] = SU_PATH;
-        command_string += SU_PATH;
-        args[1] = "root";
-        command_string += " root ";
-    }
-    int i = starting_index;
-    for (auto arg = full_command.begin(); arg != full_command.end(); ++arg) {
-        args[i++] = arg->c_str();
-        command_string += arg->c_str();
-        if (arg != full_command.end() - 1) {
-            command_string += " ";
-        }
-    }
-    args[i] = nullptr;
-    const char* path = args[0];
-    const char* command = command_string.c_str();
-
-    if (options.RootMode() == SU_ROOT && ds.IsUserBuild()) {
-        printf("Skipping '%s' on user build.\n", command);
-        return 0;
-    }
-
-    if (!title.empty()) {
-        printf("------ %s (%s) ------\n", title.c_str(), command);
-    }
-
-    fflush(stdout);
     DurationReporter duration_reporter(title);
 
-    const std::string& logging_message = options.LoggingMessage();
-    if (!logging_message.empty()) {
-        MYLOGI(logging_message.c_str(), command_string.c_str());
-    }
-
-    if (IsDryRun() && !options.Always()) {
-        if (!title.empty()) {
-            printf("\t(skipped on dry run)\n");
-        }
-        UpdateProgress(options.Timeout());
-        return 0;
-    }
-
-    int status = JustRunCommand(command, path, args, options);
+    int status = RunCommandToFd(STDOUT_FILENO, title, full_command, options);
 
     /* TODO: for now we're simplifying the progress calculation by using the
      * timeout as the weight. It's a good approximation for most cases, except when calling dumpsys,
      * where its weight should be much higher proportionally to its timeout.
      * Ideally, it should use a options.EstimatedDuration() instead...*/
-    int weight = options.Timeout();
+    UpdateProgress(options.Timeout());
 
-    if (weight > 0) {
-        UpdateProgress(weight);
-    }
-
-    return status;
-}
-
-int Dumpstate::JustRunCommand(const char* command, const char* path, std::vector<const char*>& args,
-                              const CommandOptions& options) const {
-    bool silent = (options.StdoutMode() == REDIRECT_TO_STDERR);
-
-    uint64_t start = DurationReporter::Nanotime();
-    pid_t pid = fork();
-
-    /* handle error case */
-    if (pid < 0) {
-        if (!silent) printf("*** fork: %s\n", strerror(errno));
-        MYLOGE("*** fork: %s\n", strerror(errno));
-        return pid;
-    }
-
-    /* handle child case */
-    if (pid == 0) {
-        if (options.RootMode() == DROP_ROOT && !drop_root_user()) {
-            if (!silent)
-                printf("*** failed to drop root before running %s: %s\n", command, strerror(errno));
-            MYLOGE("*** could not drop root before running %s: %s\n", command, strerror(errno));
-            return -1;
-        }
-
-        if (silent) {
-            // Redirect stderr to stdout
-            dup2(STDERR_FILENO, STDOUT_FILENO);
-        }
-
-        /* make sure the child dies when dumpstate dies */
-        prctl(PR_SET_PDEATHSIG, SIGKILL);
-
-        /* just ignore SIGPIPE, will go down with parent's */
-        struct sigaction sigact;
-        memset(&sigact, 0, sizeof(sigact));
-        sigact.sa_handler = SIG_IGN;
-        sigaction(SIGPIPE, &sigact, NULL);
-
-        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));
-        fflush(stdout);
-        // Must call _exit (instead of exit), otherwise it will corrupt the zip
-        // file.
-        _exit(EXIT_FAILURE);
-    }
-
-    /* handle parent case */
-    int status;
-    bool ret = waitpid_with_timeout(pid, options.Timeout(), &status);
-    uint64_t elapsed = DurationReporter::Nanotime() - start;
-    if (!ret) {
-        if (errno == ETIMEDOUT) {
-            if (!silent)
-                printf("*** command '%s' timed out after %.3fs (killing pid %d)\n", command,
-                       (float)elapsed / NANOS_PER_SEC, pid);
-            MYLOGE("*** command '%s' timed out after %.3fs (killing pid %d)\n", command,
-                   (float)elapsed / NANOS_PER_SEC, pid);
-        } else {
-            if (!silent)
-                printf("*** command '%s': Error after %.4fs (killing pid %d)\n", command,
-                       (float)elapsed / NANOS_PER_SEC, pid);
-            MYLOGE("command '%s': Error after %.4fs (killing pid %d)\n", command,
-                   (float)elapsed / NANOS_PER_SEC, pid);
-        }
-        kill(pid, SIGTERM);
-        if (!waitpid_with_timeout(pid, 5, nullptr)) {
-            kill(pid, SIGKILL);
-            if (!waitpid_with_timeout(pid, 5, nullptr)) {
-                if (!silent)
-                    printf("could not kill command '%s' (pid %d) even with SIGKILL.\n", command,
-                           pid);
-                MYLOGE("could not kill command '%s' (pid %d) even with SIGKILL.\n", command, pid);
-            }
-        }
-        return -1;
-    }
-
-    if (WIFSIGNALED(status)) {
-        if (!silent)
-            printf("*** command '%s' failed: killed by signal %d\n", command, WTERMSIG(status));
-        MYLOGE("*** command '%s' failed: killed by signal %d\n", command, WTERMSIG(status));
-    } else if (WIFEXITED(status) && WEXITSTATUS(status) > 0) {
-        status = WEXITSTATUS(status);
-        if (!silent) printf("*** command '%s' failed: exit code %d\n", command, status);
-        MYLOGE("*** command '%s' failed: exit code %d\n", command, status);
-    }
+    fflush(stdout);
 
     return status;
 }
@@ -1077,55 +687,6 @@
     RunCommand(title, dumpsys, options);
 }
 
-bool drop_root_user() {
-    if (getgid() == AID_SHELL && getuid() == AID_SHELL) {
-        MYLOGD("drop_root_user(): already running as Shell\n");
-        return true;
-    }
-    /* ensure we will keep capabilities when we drop root */
-    if (prctl(PR_SET_KEEPCAPS, 1) < 0) {
-        MYLOGE("prctl(PR_SET_KEEPCAPS) failed: %s\n", strerror(errno));
-        return false;
-    }
-
-    gid_t groups[] = { AID_LOG, AID_SDCARD_R, AID_SDCARD_RW,
-            AID_MOUNT, AID_INET, AID_NET_BW_STATS, AID_READPROC, AID_WAKELOCK,
-            AID_BLUETOOTH };
-    if (setgroups(sizeof(groups)/sizeof(groups[0]), groups) != 0) {
-        MYLOGE("Unable to setgroups, aborting: %s\n", strerror(errno));
-        return false;
-    }
-    if (setgid(AID_SHELL) != 0) {
-        MYLOGE("Unable to setgid, aborting: %s\n", strerror(errno));
-        return false;
-    }
-    if (setuid(AID_SHELL) != 0) {
-        MYLOGE("Unable to setuid, aborting: %s\n", strerror(errno));
-        return false;
-    }
-
-    struct __user_cap_header_struct capheader;
-    struct __user_cap_data_struct capdata[2];
-    memset(&capheader, 0, sizeof(capheader));
-    memset(&capdata, 0, sizeof(capdata));
-    capheader.version = _LINUX_CAPABILITY_VERSION_3;
-    capheader.pid = 0;
-
-    capdata[CAP_TO_INDEX(CAP_SYSLOG)].permitted =
-            (CAP_TO_MASK(CAP_SYSLOG) | CAP_TO_MASK(CAP_BLOCK_SUSPEND));
-    capdata[CAP_TO_INDEX(CAP_SYSLOG)].effective =
-            (CAP_TO_MASK(CAP_SYSLOG) | CAP_TO_MASK(CAP_BLOCK_SUSPEND));
-    capdata[0].inheritable = 0;
-    capdata[1].inheritable = 0;
-
-    if (capset(&capheader, &capdata[0]) < 0) {
-        MYLOGE("capset failed: %s\n", strerror(errno));
-        return false;
-    }
-
-    return true;
-}
-
 void send_broadcast(const std::string& action, const std::vector<std::string>& args) {
     std::vector<std::string> am = {"/system/bin/am", "broadcast", "--user", "0", "-a", action};
 
@@ -1160,7 +721,7 @@
     const char* title = "SYSTEM PROPERTIES";
     DurationReporter duration_reporter(title);
     printf("------ %s ------\n", title);
-    if (IsDryRun()) return;
+    if (PropertiesHelper::IsDryRun()) return;
     size_t i;
     num_props = 0;
     property_list(print_prop, NULL);
@@ -1347,7 +908,7 @@
             }
 
             ++dalvik_found;
-            uint64_t start = DurationReporter::Nanotime();
+            uint64_t start = Nanotime();
             if (kill(pid, SIGQUIT)) {
                 MYLOGE("kill(%d, SIGQUIT): %s\n", pid, strerror(errno));
                 continue;
@@ -1369,7 +930,7 @@
                 MYLOGE("lseek: %s\n", strerror(errno));
             } else {
                 dprintf(fd, "[dump dalvik stack %d: %.3fs elapsed]\n", pid,
-                        (float)(DurationReporter::Nanotime() - start) / NANOS_PER_SEC);
+                        (float)(Nanotime() - start) / NANOS_PER_SEC);
             }
         } else if (should_dump_native_traces(data)) {
             /* dump native process if appropriate */
@@ -1377,7 +938,7 @@
                 MYLOGE("lseek: %s\n", strerror(errno));
             } else {
                 static uint16_t timeout_failures = 0;
-                uint64_t start = DurationReporter::Nanotime();
+                uint64_t start = Nanotime();
 
                 /* If 3 backtrace dumps fail in a row, consider debuggerd dead. */
                 if (timeout_failures == 3) {
@@ -1389,7 +950,7 @@
                     timeout_failures = 0;
                 }
                 dprintf(fd, "[dump native stack %d: %.3fs elapsed]\n", pid,
-                        (float)(DurationReporter::Nanotime() - start) / NANOS_PER_SEC);
+                        (float)(Nanotime() - start) / NANOS_PER_SEC);
             }
         }
     }
@@ -1419,7 +980,7 @@
 
 void dump_route_tables() {
     DurationReporter duration_reporter("DUMP ROUTE TABLES");
-    if (IsDryRun()) return;
+    if (PropertiesHelper::IsDryRun()) return;
     const char* const RT_TABLES_PATH = "/data/misc/net/rt_tables";
     ds.DumpFile("RT_TABLES", RT_TABLES_PATH);
     FILE* fp = fopen(RT_TABLES_PATH, "re");
diff --git a/cmds/flatland/GLHelper.cpp b/cmds/flatland/GLHelper.cpp
index ddf3aa8..5c04f6c 100644
--- a/cmds/flatland/GLHelper.cpp
+++ b/cmds/flatland/GLHelper.cpp
@@ -365,6 +365,7 @@
     if (!result) {
         fprintf(stderr, "Shader source:\n");
         printShaderSource(lines);
+        delete[] src;
         return false;
     }
     delete[] src;
diff --git a/cmds/installd/commands.cpp b/cmds/installd/commands.cpp
index 5ffc0c2..9f19597 100644
--- a/cmds/installd/commands.cpp
+++ b/cmds/installd/commands.cpp
@@ -78,9 +78,11 @@
 static constexpr int FLAG_CLEAR_CODE_CACHE_ONLY = 1 << 9;
 
 /* dexopt needed flags matching those in dalvik.system.DexFile */
-static constexpr int DEXOPT_DEX2OAT_NEEDED       = 1;
-static constexpr int DEXOPT_PATCHOAT_NEEDED      = 2;
-static constexpr int DEXOPT_SELF_PATCHOAT_NEEDED = 3;
+static constexpr int DEX2OAT_FROM_SCRATCH        = 1;
+static constexpr int DEX2OAT_FOR_BOOT_IMAGE      = 2;
+static constexpr int DEX2OAT_FOR_FILTER          = 3;
+static constexpr int DEX2OAT_FOR_RELOCATION      = 4;
+static constexpr int PATCHOAT_FOR_RELOCATION     = 5;
 
 typedef int fd_t;
 
@@ -1687,20 +1689,25 @@
 
     const char *input_file;
     char in_odex_path[PKG_PATH_MAX];
-    switch (dexopt_needed) {
-        case DEXOPT_DEX2OAT_NEEDED:
+    int dexopt_action = abs(dexopt_needed);
+    bool is_odex_location = dexopt_needed < 0;
+    switch (dexopt_action) {
+        case DEX2OAT_FROM_SCRATCH:
+        case DEX2OAT_FOR_BOOT_IMAGE:
+        case DEX2OAT_FOR_FILTER:
+        case DEX2OAT_FOR_RELOCATION:
             input_file = apk_path;
             break;
 
-        case DEXOPT_PATCHOAT_NEEDED:
-            if (!calculate_odex_file_path(in_odex_path, apk_path, instruction_set)) {
-                return -1;
+        case PATCHOAT_FOR_RELOCATION:
+            if (is_odex_location) {
+                if (!calculate_odex_file_path(in_odex_path, apk_path, instruction_set)) {
+                    return -1;
+                }
+                input_file = in_odex_path;
+            } else {
+                input_file = out_oat_path;
             }
-            input_file = in_odex_path;
-            break;
-
-        case DEXOPT_SELF_PATCHOAT_NEEDED:
-            input_file = out_oat_path;
             break;
 
         default:
@@ -1737,8 +1744,7 @@
     // unlink the old one.
     base::unique_fd in_vdex_fd;
     std::string in_vdex_path_str;
-    if (dexopt_needed == DEXOPT_PATCHOAT_NEEDED
-        || dexopt_needed == DEXOPT_SELF_PATCHOAT_NEEDED) {
+    if (dexopt_action == PATCHOAT_FOR_RELOCATION) {
         // `input_file` is the OAT file to be relocated. The VDEX has to be there as well.
         in_vdex_path_str = create_vdex_filename(input_file);
         if (in_vdex_path_str.empty()) {
@@ -1751,27 +1757,25 @@
                 in_vdex_path_str.c_str(), strerror(errno));
             return -1;
         }
-    } else {
-        // Open the possibly existing vdex in the `out_oat_path`. If none exist, we pass -1
-        // to dex2oat for input-vdex-fd.
-        in_vdex_path_str = create_vdex_filename(out_oat_path);
+    } else if (dexopt_action != DEX2OAT_FROM_SCRATCH) {
+        // Open the possibly existing vdex. If none exist, we pass -1 to dex2oat for input-vdex-fd.
+        const char* path = nullptr;
+        if (is_odex_location) {
+            if (calculate_odex_file_path(in_odex_path, apk_path, instruction_set)) {
+                path = in_odex_path;
+            } else {
+                ALOGE("installd cannot compute input vdex location for '%s'\n", apk_path);
+                return -1;
+            }
+        } else {
+            path = out_oat_path;
+        }
+        in_vdex_path_str = create_vdex_filename(path);
         if (in_vdex_path_str.empty()) {
-            ALOGE("installd cannot compute input vdex location for '%s'\n", out_oat_path);
+            ALOGE("installd cannot compute input vdex location for '%s'\n", path);
             return -1;
         }
         in_vdex_fd.reset(open(in_vdex_path_str.c_str(), O_RDONLY, 0));
-        // If there is no vdex file in out_oat_path, check if we have a vdex
-        // file next to the odex file. For other failures, we will just pass a -1 fd.
-        if (in_vdex_fd.get() < 0 && (errno == ENOENT) && IsOutputDalvikCache(oat_dir)) {
-            if (calculate_odex_file_path(in_odex_path, apk_path, instruction_set)) {
-              in_vdex_path_str = create_vdex_filename(std::string(in_odex_path));
-              if (in_vdex_path_str.empty()) {
-                  ALOGE("installd cannot compute input vdex location for '%s'\n", in_odex_path);
-                  return -1;
-              }
-              in_vdex_fd.reset(open(in_vdex_path_str.c_str(), O_RDONLY, 0));
-            }
-        }
     }
 
     // Infer the name of the output VDEX and create it.
@@ -1815,7 +1819,7 @@
     // Avoid generating an app image for extract only since it will not contain any classes.
     Dex2oatFileWrapper<std::function<void ()>> image_fd;
     const std::string image_path = create_image_filename(out_oat_path);
-    if (dexopt_needed == DEXOPT_DEX2OAT_NEEDED && !image_path.empty()) {
+    if (dexopt_action != PATCHOAT_FOR_RELOCATION && !image_path.empty()) {
         char app_image_format[kPropertyValueMax];
         bool have_app_image_format =
                 get_property("dalvik.vm.appimageformat", app_image_format, NULL) > 0;
@@ -1865,8 +1869,7 @@
             _exit(67);
         }
 
-        if (dexopt_needed == DEXOPT_PATCHOAT_NEEDED
-            || dexopt_needed == DEXOPT_SELF_PATCHOAT_NEEDED) {
+        if (dexopt_action == PATCHOAT_FOR_RELOCATION) {
             run_patchoat(input_fd.get(),
                          in_vdex_fd.get(),
                          out_oat_fd.get(),
@@ -1877,7 +1880,7 @@
                          out_vdex_path_str.c_str(),
                          pkgname,
                          instruction_set);
-        } else if (dexopt_needed == DEXOPT_DEX2OAT_NEEDED) {
+        } else {
             // Pass dex2oat the relative path to the input file.
             const char *input_file_name = get_location_from_path(input_file);
             run_dex2oat(input_fd.get(),
@@ -1895,9 +1898,6 @@
                         boot_complete,
                         reference_profile_fd.get(),
                         shared_libraries);
-        } else {
-            ALOGE("Invalid dexopt needed: %d\n", dexopt_needed);
-            _exit(73);
         }
         _exit(68);   /* only get here on exec failure */
     } else {
diff --git a/include/binder/AppOpsManager.h b/include/binder/AppOpsManager.h
index 042927c..4212776 100644
--- a/include/binder/AppOpsManager.h
+++ b/include/binder/AppOpsManager.h
@@ -91,7 +91,8 @@
         OP_USE_SIP = 53,
         OP_PROCESS_OUTGOING_CALLS = 54,
         OP_USE_FINGERPRINT = 55,
-        OP_BODY_SENSORS = 56
+        OP_BODY_SENSORS = 56,
+        OP_AUDIO_ACCESSIBILITY_VOLUME = 64,
     };
 
     AppOpsManager();
diff --git a/include/binder/Parcel.h b/include/binder/Parcel.h
index 39deb64..69de136 100644
--- a/include/binder/Parcel.h
+++ b/include/binder/Parcel.h
@@ -697,8 +697,16 @@
         return UNEXPECTED_NULL;
     }
 
+    if (val->max_size() < static_cast<size_t>(size)) {
+        return NO_MEMORY;
+    }
+
     val->resize(static_cast<size_t>(size));
 
+    if (val->size() < static_cast<size_t>(size)) {
+        return NO_MEMORY;
+    }
+
     for (auto& v: *val) {
         status = (this->*read_func)(&v);
 
diff --git a/include/media/openmax/OMX_AsString.h b/include/media/openmax/OMX_AsString.h
index b18fd54..4556c8f 100644
--- a/include/media/openmax/OMX_AsString.h
+++ b/include/media/openmax/OMX_AsString.h
@@ -534,6 +534,8 @@
         case OMX_IndexParamAudioAndroidAc3:             return "ParamAudioAndroidAc3";
         case OMX_IndexParamAudioAndroidOpus:            return "ParamAudioAndroidOpus";
         case OMX_IndexParamAudioAndroidAacPresentation: return "ParamAudioAndroidAacPresentation";
+        case OMX_IndexParamAudioAndroidEac3:            return "ParamAudioAndroidEac3";
+        case OMX_IndexParamAudioProfileQuerySupported:  return "ParamAudioProfileQuerySupported";
 //      case OMX_IndexParamNalStreamFormatSupported:    return "ParamNalStreamFormatSupported";
 //      case OMX_IndexParamNalStreamFormat:             return "ParamNalStreamFormat";
 //      case OMX_IndexParamNalStreamFormatSelect:       return "ParamNalStreamFormatSelect";
@@ -548,6 +550,8 @@
         case OMX_IndexConfigAndroidVideoTemporalLayering: return "ConfigAndroidVideoTemporalLayering";
         case OMX_IndexParamMaxFrameDurationForBitrateControl:
             return "ParamMaxFrameDurationForBitrateControl";
+        case OMX_IndexParamVideoVp9:                    return "ParamVideoVp9";
+        case OMX_IndexParamVideoAndroidVp9Encoder:      return "ParamVideoAndroidVp9Encoder";
         case OMX_IndexConfigAutoFramerateConversion:    return "ConfigAutoFramerateConversion";
         case OMX_IndexConfigPriority:                   return "ConfigPriority";
         case OMX_IndexConfigOperatingRate:              return "ConfigOperatingRate";
diff --git a/include/media/openmax/OMX_IndexExt.h b/include/media/openmax/OMX_IndexExt.h
index 63fbff8..d0ae867 100644
--- a/include/media/openmax/OMX_IndexExt.h
+++ b/include/media/openmax/OMX_IndexExt.h
@@ -76,14 +76,14 @@
     OMX_IndexConfigVideoVp8ReferenceFrame,          /**< reference: OMX_VIDEO_VP8REFERENCEFRAMETYPE */
     OMX_IndexConfigVideoVp8ReferenceFrameType,      /**< reference: OMX_VIDEO_VP8REFERENCEFRAMEINFOTYPE */
     OMX_IndexParamVideoAndroidVp8Encoder,           /**< reference: OMX_VIDEO_PARAM_ANDROID_VP8ENCODERTYPE */
-    OMX_IndexParamVideoVp9,                         /**< reference: OMX_VIDEO_PARAM_VP9TYPE */
-    OMX_IndexParamVideoAndroidVp9Encoder,           /**< reference: OMX_VIDEO_PARAM_ANDROID_VP9ENCODERTYPE */
     OMX_IndexParamVideoHevc,                        /**< reference: OMX_VIDEO_PARAM_HEVCTYPE */
     OMX_IndexParamSliceSegments,                    /**< reference: OMX_VIDEO_SLICESEGMENTSTYPE */
     OMX_IndexConfigAndroidIntraRefresh,             /**< reference: OMX_VIDEO_CONFIG_ANDROID_INTRAREFRESHTYPE */
     OMX_IndexParamAndroidVideoTemporalLayering,     /**< reference: OMX_VIDEO_PARAM_ANDROID_TEMPORALLAYERINGTYPE */
     OMX_IndexConfigAndroidVideoTemporalLayering,    /**< reference: OMX_VIDEO_CONFIG_ANDROID_TEMPORALLAYERINGTYPE */
     OMX_IndexParamMaxFrameDurationForBitrateControl,/**< reference: OMX_PARAM_U32TYPE */
+    OMX_IndexParamVideoVp9,                         /**< reference: OMX_VIDEO_PARAM_VP9TYPE */
+    OMX_IndexParamVideoAndroidVp9Encoder,           /**< reference: OMX_VIDEO_PARAM_ANDROID_VP9ENCODERTYPE */
     OMX_IndexExtVideoEndUnused,
 
     /* Image & Video common configurations */
diff --git a/include/ui/ColorSpace.h b/include/ui/ColorSpace.h
index 2543e7f..c6a20c9 100644
--- a/include/ui/ColorSpace.h
+++ b/include/ui/ColorSpace.h
@@ -99,11 +99,10 @@
     /**
      * Converts the supplied RGB value to XYZ. The input RGB value
      * is decoded using this color space's electro-optical function
-     * before being converted to XYZ. The returned result is clamped
-     * by this color space's clamping function.
+     * before being converted to XYZ.
      */
     constexpr float3 rgbToXYZ(const float3& rgb) const noexcept {
-        return apply(mRGBtoXYZ * toLinear(rgb), mClamper);
+        return mRGBtoXYZ * toLinear(rgb);
     }
 
     constexpr const std::string& getName() const noexcept {
diff --git a/include/ui/GrallocAllocator.h b/include/ui/GrallocAllocator.h
index 23b78ed..5645bed 100644
--- a/include/ui/GrallocAllocator.h
+++ b/include/ui/GrallocAllocator.h
@@ -27,12 +27,13 @@
 namespace Gralloc2 {
 
 using hardware::graphics::allocator::V2_0::Error;
-using hardware::graphics::allocator::V2_0::PixelFormat;
 using hardware::graphics::allocator::V2_0::ProducerUsage;
 using hardware::graphics::allocator::V2_0::ConsumerUsage;
 using hardware::graphics::allocator::V2_0::BufferDescriptor;
 using hardware::graphics::allocator::V2_0::Buffer;
 using hardware::graphics::allocator::V2_0::IAllocator;
+using hardware::graphics::allocator::V2_0::IAllocatorClient;
+using hardware::graphics::common::V1_0::PixelFormat;
 
 // Allocator is a wrapper to IAllocator, a proxy to server-side allocator.
 class Allocator {
@@ -40,12 +41,12 @@
     Allocator();
 
     // this will be removed and Allocator will be always valid
-    bool valid() const { return (mService != nullptr); }
+    bool valid() const { return (mAllocator != nullptr); }
 
     std::string dumpDebugInfo() const;
 
     Error createBufferDescriptor(
-            const IAllocator::BufferDescriptorInfo& descriptorInfo,
+            const IAllocatorClient::BufferDescriptorInfo& descriptorInfo,
             BufferDescriptor& descriptor) const;
     void destroyBufferDescriptor(BufferDescriptor descriptor) const;
 
@@ -56,7 +57,8 @@
             native_handle_t*& bufferHandle) const;
 
 private:
-    sp<IAllocator> mService;
+    sp<IAllocator> mAllocator;
+    sp<IAllocatorClient> mClient;
 };
 
 } // namespace Gralloc2
diff --git a/include/ui/GrallocMapper.h b/include/ui/GrallocMapper.h
index 868fd14..99d2837 100644
--- a/include/ui/GrallocMapper.h
+++ b/include/ui/GrallocMapper.h
@@ -27,9 +27,9 @@
 namespace Gralloc2 {
 
 using hardware::graphics::allocator::V2_0::Error;
-using hardware::graphics::allocator::V2_0::PixelFormat;
 using hardware::graphics::allocator::V2_0::ProducerUsage;
 using hardware::graphics::allocator::V2_0::ConsumerUsage;
+using hardware::graphics::common::V1_0::PixelFormat;
 using hardware::graphics::mapper::V2_0::FlexLayout;
 using hardware::graphics::mapper::V2_0::BackingStore;
 using hardware::graphics::mapper::V2_0::Device;
diff --git a/include/ui/mat3.h b/include/ui/mat3.h
index cd24a44..4f5dba9 100644
--- a/include/ui/mat3.h
+++ b/include/ui/mat3.h
@@ -292,7 +292,7 @@
     m_value[2] = col_type(0, 0, v.z);
 }
 
-// construct from 16 scalars. Note that the arrangement
+// construct from 9 scalars. Note that the arrangement
 // of values in the constructor is the transpose of the matrix
 // notation.
 template<typename T>
diff --git a/libs/binder/tests/binderThroughputTest.cpp b/libs/binder/tests/binderThroughputTest.cpp
index 71b96d4..6e8f7df 100644
--- a/libs/binder/tests/binderThroughputTest.cpp
+++ b/libs/binder/tests/binderThroughputTest.cpp
@@ -170,6 +170,8 @@
     int num,
     int worker_count,
     int iterations,
+    int payload_size,
+    bool cs_pair,
     Pipe p)
 {
     // Create BinderWorkerService and for go.
@@ -182,22 +184,32 @@
     p.signal();
     p.wait();
 
+    // If client/server pairs, then half the workers are
+    // servers and half are clients
+    int server_count = cs_pair ? worker_count / 2 : worker_count;
+
     // Get references to other binder services.
     cout << "Created BinderWorker" << num << endl;
     (void)worker_count;
     vector<sp<IBinder> > workers;
-    for (int i = 0; i < worker_count; i++) {
+    for (int i = 0; i < server_count; i++) {
         if (num == i)
             continue;
         workers.push_back(serviceMgr->getService(generateServiceName(i)));
     }
 
-    // Run the benchmark.
+    // Run the benchmark if client
     ProcResults results;
     chrono::time_point<chrono::high_resolution_clock> start, end;
-    for (int i = 0; i < iterations; i++) {
-        int target = rand() % workers.size();
+    for (int i = 0; (!cs_pair || num >= server_count) && i < iterations; i++) {
         Parcel data, reply;
+        int target = cs_pair ? num % server_count : rand() % workers.size();
+	int sz = payload_size;
+
+	while (sz > sizeof(uint32_t)) {
+		data.writeInt32(0);
+		sz -= sizeof(uint32_t);
+	}
         start = chrono::high_resolution_clock::now();
         status_t ret = workers[target]->transact(BINDER_NOP, data, &reply);
         end = chrono::high_resolution_clock::now();
@@ -210,6 +222,7 @@
            exit(EXIT_FAILURE);
         }
     }
+
     // Signal completion to master and wait.
     p.signal();
     p.wait();
@@ -221,7 +234,7 @@
     exit(EXIT_SUCCESS);
 }
 
-Pipe make_worker(int num, int iterations, int worker_count)
+Pipe make_worker(int num, int iterations, int worker_count, int payload_size, bool cs_pair)
 {
     auto pipe_pair = Pipe::createPipePair();
     pid_t pid = fork();
@@ -230,7 +243,7 @@
         return move(get<0>(pipe_pair));
     } else {
         /* child */
-        worker_fx(num, worker_count, iterations, move(get<1>(pipe_pair)));
+        worker_fx(num, worker_count, iterations, payload_size, cs_pair, move(get<1>(pipe_pair)));
         /* never get here */
         return move(get<0>(pipe_pair));
     }
@@ -255,6 +268,8 @@
 {
     int workers = 2;
     int iterations = 10000;
+    int payload_size = 0;
+    bool cs_pair = false;
     (void)argc;
     (void)argv;
     vector<Pipe> pipes;
@@ -271,11 +286,21 @@
             i++;
             continue;
         }
+        if (string(argv[i]) == "-s") {
+            payload_size = atoi(argv[i+1]);
+	    i++;
+	}
+        if (string(argv[i]) == "-p") {
+		// client/server pairs instead of spreading
+		// requests to all workers. If true, half
+		// the workers become clients and half servers
+		cs_pair = true;
+	}
     }
 
     // Create all the workers and wait for them to spawn.
     for (int i = 0; i < workers; i++) {
-        pipes.push_back(make_worker(i, iterations, workers));
+        pipes.push_back(make_worker(i, iterations, workers, payload_size, cs_pair));
     }
     wait_all(pipes);
 
diff --git a/libs/ui/Android.bp b/libs/ui/Android.bp
index 087e877..0adfdea 100644
--- a/libs/ui/Android.bp
+++ b/libs/ui/Android.bp
@@ -67,7 +67,8 @@
         "libbinder",
         "libcutils",
         "libhardware",
-        "libhidl",
+        "libhidlbase",
+        "libhidltransport",
         "libsync",
         "libutils",
         "liblog",
diff --git a/libs/ui/ColorSpace.cpp b/libs/ui/ColorSpace.cpp
index 4ea3b96..1a60f0f 100644
--- a/libs/ui/ColorSpace.cpp
+++ b/libs/ui/ColorSpace.cpp
@@ -97,7 +97,7 @@
 }
 
 static constexpr float rcpResponse(float x, float g,float a, float b, float c, float d) {
-    return x >= d * c ? std::pow(x / a, 1.0f / g) - b / a : x / c;
+    return x >= d * c ? (std::pow(x, 1.0f / g) - b) / a : x / c;
 }
 
 static constexpr float response(float x, float g, float a, float b, float c, float d) {
@@ -137,7 +137,7 @@
         {0.3127f, 0.3290f},
         std::bind(absRcpResponse, _1, 2.4f, 1 / 1.055f, 0.055f / 1.055f, 1 / 12.92f, 0.04045f),
         std::bind(absResponse,    _1, 2.4f, 1 / 1.055f, 0.055f / 1.055f, 1 / 12.92f, 0.04045f),
-        [](float x){return x;}
+        std::bind(clamp<float>, _1, -0.5f, 7.5f)
     };
 }
 
@@ -148,7 +148,7 @@
         {0.3127f, 0.3290f},
         linearReponse,
         linearReponse,
-        [](float x){return x;}
+        std::bind(clamp<float>, _1, -0.5f, 7.5f)
     };
 }
 
@@ -187,8 +187,8 @@
         "Adobe RGB (1998)",
         {{float2{0.64f, 0.33f}, {0.21f, 0.71f}, {0.15f, 0.06f}}},
         {0.3127f, 0.3290f},
-        std::bind(saturate<float>, std::bind(powf, _1, 1.0f / 2.2f)),
-        std::bind(saturate<float>, std::bind(powf, _1, 2.2f))
+        std::bind(powf, _1, 1.0f / 2.2f),
+        std::bind(powf, _1, 2.2f)
     };
 }
 
@@ -216,7 +216,7 @@
     return {
         "SMPTE RP 431-2-2007 DCI (P3)",
         {{float2{0.680f, 0.320f}, {0.265f, 0.690f}, {0.150f, 0.060f}}},
-        {0.3127f, 0.3290f},
+        {0.314f, 0.351f},
         std::bind(powf, _1, 1.0f / 2.6f),
         std::bind(powf, _1, 2.6f)
     };
@@ -226,7 +226,10 @@
     return {
         "SMPTE ST 2065-1:2012 ACES",
         {{float2{0.73470f, 0.26530f}, {0.0f, 1.0f}, {0.00010f, -0.0770f}}},
-        {0.32168f, 0.33767f}
+        {0.32168f, 0.33767f},
+        linearReponse,
+        linearReponse,
+        std::bind(clamp<float>, _1, -65504.0f, 65504.0f)
     };
 }
 
@@ -234,7 +237,10 @@
     return {
         "Academy S-2014-004 ACEScg",
         {{float2{0.713f, 0.293f}, {0.165f, 0.830f}, {0.128f, 0.044f}}},
-        {0.32168f, 0.33767f}
+        {0.32168f, 0.33767f},
+        linearReponse,
+        linearReponse,
+        std::bind(clamp<float>, _1, -65504.0f, 65504.0f)
     };
 }
 
diff --git a/libs/ui/GrallocAllocator.cpp b/libs/ui/GrallocAllocator.cpp
index 2eb1988..021122a 100644
--- a/libs/ui/GrallocAllocator.cpp
+++ b/libs/ui/GrallocAllocator.cpp
@@ -28,14 +28,25 @@
 
 Allocator::Allocator()
 {
-    mService = IAllocator::getService("gralloc");
+    mAllocator = IAllocator::getService("gralloc");
+    if (mAllocator != nullptr) {
+        mAllocator->createClient(
+                [&](const auto& tmpError, const auto& tmpClient) {
+                    if (tmpError == Error::NONE) {
+                        mClient = tmpClient;
+                    }
+                });
+        if (mClient == nullptr) {
+            mAllocator.clear();
+        }
+    }
 }
 
 std::string Allocator::dumpDebugInfo() const
 {
     std::string info;
 
-    mService->dumpDebugInfo([&](const auto& tmpInfo) {
+    mAllocator->dumpDebugInfo([&](const auto& tmpInfo) {
         info = tmpInfo.c_str();
     });
 
@@ -43,11 +54,11 @@
 }
 
 Error Allocator::createBufferDescriptor(
-        const IAllocator::BufferDescriptorInfo& descriptorInfo,
+        const IAllocatorClient::BufferDescriptorInfo& descriptorInfo,
         BufferDescriptor& descriptor) const
 {
     Error error = kDefaultError;
-    mService->createDescriptor(descriptorInfo,
+    mClient->createDescriptor(descriptorInfo,
             [&](const auto& tmpError, const auto& tmpDescriptor) {
                 error = tmpError;
                 if (error != Error::NONE) {
@@ -62,7 +73,7 @@
 
 void Allocator::destroyBufferDescriptor(BufferDescriptor descriptor) const
 {
-    mService->destroyDescriptor(descriptor);
+    mClient->destroyDescriptor(descriptor);
 }
 
 Error Allocator::allocate(BufferDescriptor descriptor, Buffer& buffer) const
@@ -71,7 +82,7 @@
     descriptors.setToExternal(&descriptor, 1);
 
     Error error = kDefaultError;
-    auto status = mService->allocate(descriptors,
+    auto status = mClient->allocate(descriptors,
             [&](const auto& tmpError, const auto& tmpBuffers) {
                 error = tmpError;
                 if (tmpError != Error::NONE) {
@@ -86,14 +97,14 @@
 
 void Allocator::free(Buffer buffer) const
 {
-    mService->free(buffer);
+    mClient->free(buffer);
 }
 
 Error Allocator::exportHandle(BufferDescriptor descriptor, Buffer buffer,
         native_handle_t*& bufferHandle) const
 {
     Error error = kDefaultError;
-    auto status = mService->exportHandle(descriptor, buffer,
+    auto status = mClient->exportHandle(descriptor, buffer,
             [&](const auto& tmpError, const auto& tmpBufferHandle) {
                 error = tmpError;
                 if (tmpError != Error::NONE) {
diff --git a/libs/ui/GraphicBufferAllocator.cpp b/libs/ui/GraphicBufferAllocator.cpp
index e333bc1..d258586 100644
--- a/libs/ui/GraphicBufferAllocator.cpp
+++ b/libs/ui/GraphicBufferAllocator.cpp
@@ -106,7 +106,7 @@
             PixelFormat format, uint32_t layerCount, uint32_t usage)
         : mAllocator(allocator), mBufferValid(false)
     {
-        Gralloc2::IAllocator::BufferDescriptorInfo info = {};
+        Gralloc2::IAllocatorClient::BufferDescriptorInfo info = {};
         info.width = width;
         info.height = height;
         info.format = static_cast<Gralloc2::PixelFormat>(format);
diff --git a/libs/ui/tests/colorspace_test.cpp b/libs/ui/tests/colorspace_test.cpp
index 5c127ad..e5c2633 100644
--- a/libs/ui/tests/colorspace_test.cpp
+++ b/libs/ui/tests/colorspace_test.cpp
@@ -106,6 +106,11 @@
 TEST_F(ColorSpaceTest, TransferFunctions) {
     ColorSpace sRGB = ColorSpace::sRGB();
 
+    EXPECT_NEAR(0.0f, sRGB.getEOTF()(0.0f), 1e-6f);
+    EXPECT_NEAR(0.0f, sRGB.getOETF()(0.0f), 1e-6f);
+    EXPECT_NEAR(1.0f, sRGB.getEOTF()(1.0f), 1e-6f);
+    EXPECT_NEAR(1.0f, sRGB.getOETF()(1.0f), 1e-6f);
+
     for (float v = 0.0f; v <= 0.5f; v += 1e-3f) {
         ASSERT_TRUE(v >= sRGB.getEOTF()(v));
         ASSERT_TRUE(v <= sRGB.getOETF()(v));
@@ -134,7 +139,7 @@
 
 TEST_F(ColorSpaceTest, Clamping) {
     // Pick a color outside of sRGB
-    float3 c(ColorSpace::DCIP3().rgbToXYZ(float3{0, 1, 0}));
+    float3 c(ColorSpace::BT2020().rgbToXYZ(float3{0, 1, 0}));
 
     // The color will be clamped
     float3 sRGB(ColorSpace::sRGB().xyzToRGB(c));
diff --git a/opengl/include/EGL/egl.h b/opengl/include/EGL/egl.h
index 99ea342..ccb54ea 100644
--- a/opengl/include/EGL/egl.h
+++ b/opengl/include/EGL/egl.h
@@ -65,13 +65,13 @@
 #define EGL_TRUE			1
 
 /* Out-of-band handle values */
-#define EGL_DEFAULT_DISPLAY		((EGLNativeDisplayType)0)
-#define EGL_NO_CONTEXT			((EGLContext)0)
-#define EGL_NO_DISPLAY			((EGLDisplay)0)
-#define EGL_NO_SURFACE			((EGLSurface)0)
+#define EGL_DEFAULT_DISPLAY		EGL_CAST(EGLNativeDisplayType, 0)
+#define EGL_NO_CONTEXT			EGL_CAST(EGLContext, 0)
+#define EGL_NO_DISPLAY			EGL_CAST(EGLDisplay, 0)
+#define EGL_NO_SURFACE			EGL_CAST(EGLSurface, 0)
 
 /* Out-of-band attribute value */
-#define EGL_DONT_CARE			((EGLint)-1)
+#define EGL_DONT_CARE			EGL_CAST(EGLint, -1)
 
 /* Errors / GetError return values */
 #define EGL_SUCCESS			0x3000
@@ -198,7 +198,7 @@
 #define EGL_DISPLAY_SCALING		10000
 
 /* Unknown display resolution/aspect ratio */
-#define EGL_UNKNOWN			((EGLint)-1)
+#define EGL_UNKNOWN			EGL_CAST(EGLint, -1)
 
 /* Back buffer swap behaviors */
 #define EGL_BUFFER_PRESERVED		0x3094	/* EGL_SWAP_BEHAVIOR value */
diff --git a/opengl/include/EGL/eglext.h b/opengl/include/EGL/eglext.h
index b70d5b2..73e5e07 100644
--- a/opengl/include/EGL/eglext.h
+++ b/opengl/include/EGL/eglext.h
@@ -79,7 +79,7 @@
 #define EGL_KHR_image 1
 #define EGL_NATIVE_PIXMAP_KHR			0x30B0	/* eglCreateImageKHR target */
 typedef void *EGLImageKHR;
-#define EGL_NO_IMAGE_KHR			((EGLImageKHR)0)
+#define EGL_NO_IMAGE_KHR			EGL_CAST(EGLImageKHR, 0)
 #ifdef EGL_EGLEXT_PROTOTYPES
 EGLAPI EGLImageKHR EGLAPIENTRY eglCreateImageKHR (EGLDisplay dpy, EGLContext ctx, EGLenum target, EGLClientBuffer buffer, const EGLint *attrib_list);
 EGLAPI EGLBoolean EGLAPIENTRY eglDestroyImageKHR (EGLDisplay dpy, EGLImageKHR image);
@@ -143,7 +143,7 @@
 #define EGL_SYNC_REUSABLE_KHR			0x30FA
 #define EGL_SYNC_FLUSH_COMMANDS_BIT_KHR		0x0001	/* eglClientWaitSyncKHR <flags> bitfield */
 #define EGL_FOREVER_KHR				0xFFFFFFFFFFFFFFFFull
-#define EGL_NO_SYNC_KHR				((EGLSyncKHR)0)
+#define EGL_NO_SYNC_KHR				EGL_CAST(EGLSyncKHR, 0)
 #ifdef EGL_EGLEXT_PROTOTYPES
 EGLAPI EGLSyncKHR EGLAPIENTRY eglCreateSyncKHR(EGLDisplay dpy, EGLenum type, const EGLint *attrib_list);
 EGLAPI EGLBoolean EGLAPIENTRY eglDestroySyncKHR(EGLDisplay dpy, EGLSyncKHR sync);
@@ -220,7 +220,7 @@
 #define EGL_SYNC_TYPE_NV			0x30ED
 #define EGL_SYNC_CONDITION_NV			0x30EE
 #define EGL_SYNC_FENCE_NV			0x30EF
-#define EGL_NO_SYNC_NV				((EGLSyncNV)0)
+#define EGL_NO_SYNC_NV				EGL_CAST(EGLSyncNV, 0)
 typedef void* EGLSyncNV;
 typedef khronos_utime_nanoseconds_t EGLTimeNV;
 #ifdef EGL_EGLEXT_PROTOTYPES
@@ -346,7 +346,7 @@
 #define EGL_KHR_stream 1
 typedef void* EGLStreamKHR;
 typedef khronos_uint64_t EGLuint64KHR;
-#define EGL_NO_STREAM_KHR			((EGLStreamKHR)0)
+#define EGL_NO_STREAM_KHR			EGL_CAST(EGLStreamKHR, 0)
 #define EGL_CONSUMER_LATENCY_USEC_KHR		0x3210
 #define EGL_PRODUCER_FRAME_KHR			0x3212
 #define EGL_CONSUMER_FRAME_KHR			0x3213
@@ -473,7 +473,7 @@
 #ifndef EGL_KHR_stream_cross_process_fd
 #define EGL_KHR_stream_cross_process_fd 1
 typedef int EGLNativeFileDescriptorKHR;
-#define EGL_NO_FILE_DESCRIPTOR_KHR		((EGLNativeFileDescriptorKHR)(-1))
+#define EGL_NO_FILE_DESCRIPTOR_KHR		EGL_CAST(EGLNativeFileDescriptorKHR, -1)
 #ifdef EGL_EGLEXT_PROTOTYPES
 EGLAPI EGLNativeFileDescriptorKHR EGLAPIENTRY eglGetStreamFileDescriptorKHR(EGLDisplay dpy, EGLStreamKHR stream);
 EGLAPI EGLStreamKHR EGLAPIENTRY eglCreateStreamFromFileDescriptorKHR(EGLDisplay dpy, EGLNativeFileDescriptorKHR file_descriptor);
@@ -615,7 +615,7 @@
 #ifdef EGL_EGLEXT_PROTOTYPES
 EGLAPI EGLClientBuffer eglCreateNativeClientBufferANDROID (const EGLint *attrib_list);
 #else
-typedef EGLAPI EGLClientBuffer (EGLAPIENTRYP PFNEGLCREATENATIVECLIENTBUFFERANDROID) (const EGLint *attrib_list);
+typedef EGLClientBuffer (EGLAPIENTRYP PFNEGLCREATENATIVECLIENTBUFFERANDROID) (const EGLint *attrib_list);
 #endif
 #endif
 
diff --git a/opengl/include/EGL/eglplatform.h b/opengl/include/EGL/eglplatform.h
index 354ac22..54011c8 100644
--- a/opengl/include/EGL/eglplatform.h
+++ b/opengl/include/EGL/eglplatform.h
@@ -121,4 +121,10 @@
  */
 typedef khronos_int32_t EGLint;
 
+#if defined(__cplusplus)
+#define EGL_CAST(type, value) (static_cast<type>(value))
+#else
+#define EGL_CAST(type, value) ((type) (value))
+#endif
+
 #endif /* __eglplatform_h */
diff --git a/opengl/tests/EGLTest/Android.mk b/opengl/tests/EGLTest/Android.mk
index 80e4867..b772450 100644
--- a/opengl/tests/EGLTest/Android.mk
+++ b/opengl/tests/EGLTest/Android.mk
@@ -17,6 +17,7 @@
 	libbinder \
 	libutils \
 	libgui \
+	libbase \
 
 LOCAL_C_INCLUDES := \
     bionic/libc/private \
diff --git a/opengl/tests/EGLTest/egl_cache_test.cpp b/opengl/tests/EGLTest/egl_cache_test.cpp
index c5bf296..c974f63 100644
--- a/opengl/tests/EGLTest/egl_cache_test.cpp
+++ b/opengl/tests/EGLTest/egl_cache_test.cpp
@@ -21,9 +21,13 @@
 
 #include <utils/Log.h>
 
+#include <android-base/test_utils.h>
+
 #include "egl_cache.h"
 #include "egl_display.h"
 
+#include <memory>
+
 namespace android {
 
 class EGLCacheTest : public ::testing::Test {
@@ -79,23 +83,20 @@
 
     virtual void SetUp() {
         EGLCacheTest::SetUp();
-
-        char* tn = tempnam("/sdcard", "EGL_test-cache-");
-        mFilename = tn;
-        free(tn);
+        mTempFile.reset(new TemporaryFile());
     }
 
     virtual void TearDown() {
-        unlink(mFilename.string());
+        mTempFile.reset(nullptr);
         EGLCacheTest::TearDown();
     }
 
-    String8 mFilename;
+    std::unique_ptr<TemporaryFile> mTempFile;
 };
 
 TEST_F(EGLCacheSerializationTest, ReinitializedCacheContainsValues) {
     uint8_t buf[4] = { 0xee, 0xee, 0xee, 0xee };
-    mCache->setCacheFilename(mFilename);
+    mCache->setCacheFilename(&mTempFile->path[0]);
     mCache->initialize(egl_display_t::get(EGL_DEFAULT_DISPLAY));
     mCache->setBlob("abcd", 4, "efgh", 4);
     mCache->terminate();
diff --git a/opengl/tools/glgen/stubs/egl/eglCreatePixmapSurface.java b/opengl/tools/glgen/stubs/egl/eglCreatePixmapSurface.java
new file mode 100644
index 0000000..bc6740e
--- /dev/null
+++ b/opengl/tools/glgen/stubs/egl/eglCreatePixmapSurface.java
@@ -0,0 +1,10 @@
+    // C function EGLSurface eglCreatePixmapSurface ( EGLDisplay dpy, EGLConfig config, EGLNativePixmapType pixmap, const EGLint *attrib_list )
+
+    @Deprecated
+    public static native EGLSurface eglCreatePixmapSurface(
+        EGLDisplay dpy,
+        EGLConfig config,
+        int pixmap,
+        int[] attrib_list,
+        int offset
+    );
\ No newline at end of file
diff --git a/opengl/tools/glgen/stubs/gles11/glGetBufferPointerv.java b/opengl/tools/glgen/stubs/gles11/glGetBufferPointerv.java
index c966e11..57338c7 100644
--- a/opengl/tools/glgen/stubs/gles11/glGetBufferPointerv.java
+++ b/opengl/tools/glgen/stubs/gles11/glGetBufferPointerv.java
@@ -1,5 +1,9 @@
     // C function void glGetBufferPointerv ( GLenum target, GLenum pname, GLvoid** params )
 
+    /**
+     * The {@link java.nio.Buffer} instance returned by this method is guaranteed
+     * to be an instance of {@link java.nio.ByteBuffer}.
+     */
     public static native java.nio.Buffer glGetBufferPointerv(
         int target,
         int pname
diff --git a/opengl/tools/glgen/stubs/gles11/glGetShaderInfoLog.cpp b/opengl/tools/glgen/stubs/gles11/glGetShaderInfoLog.cpp
index dd656b6..6d42e56 100644
--- a/opengl/tools/glgen/stubs/gles11/glGetShaderInfoLog.cpp
+++ b/opengl/tools/glgen/stubs/gles11/glGetShaderInfoLog.cpp
@@ -5,15 +5,16 @@
     GLint infoLen = 0;
     glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &infoLen);
     if (!infoLen) {
-        return _env->NewStringUTF("");
+        infoLen = 512;
     }
     char* buf = (char*) malloc(infoLen);
     if (buf == NULL) {
         jniThrowException(_env, "java/lang/IllegalArgumentException", "out of memory");
         return NULL;
     }
-    glGetShaderInfoLog(shader, infoLen, NULL, buf);
-    jstring result = _env->NewStringUTF(buf);
+    GLsizei outLen = 0;
+    glGetShaderInfoLog(shader, infoLen, &outLen, buf);
+    jstring result = _env->NewStringUTF(outLen == 0 ? "" : buf);
     free(buf);
     return result;
 }
diff --git a/opengl/tools/glgen/stubs/gles11/glMapBufferRange.java b/opengl/tools/glgen/stubs/gles11/glMapBufferRange.java
index 482ea99..7b1966b 100644
--- a/opengl/tools/glgen/stubs/gles11/glMapBufferRange.java
+++ b/opengl/tools/glgen/stubs/gles11/glMapBufferRange.java
@@ -1,5 +1,9 @@
     // C function GLvoid * glMapBufferRange ( GLenum target, GLintptr offset, GLsizeiptr length, GLbitfield access )
 
+    /**
+     * The {@link java.nio.Buffer} instance returned by this method is guaranteed
+     * to be an instance of {@link java.nio.ByteBuffer}.
+     */
     public static native java.nio.Buffer glMapBufferRange(
         int target,
         int offset,
diff --git a/services/sensorservice/Android.mk b/services/sensorservice/Android.mk
index d6e6187..86af0ef 100644
--- a/services/sensorservice/Android.mk
+++ b/services/sensorservice/Android.mk
@@ -46,7 +46,8 @@
 
 LOCAL_SHARED_LIBRARIES += \
     libbase \
-    libhidl \
+    libhidlbase \
+    libhidltransport \
     libhwbinder \
     android.hardware.sensors@1.0
 
diff --git a/services/sensorservice/BatteryService.cpp b/services/sensorservice/BatteryService.cpp
index 81f32cd..452c8c6 100644
--- a/services/sensorservice/BatteryService.cpp
+++ b/services/sensorservice/BatteryService.cpp
@@ -30,12 +30,7 @@
 namespace android {
 // ---------------------------------------------------------------------------
 
-BatteryService::BatteryService() {
-    const sp<IServiceManager> sm(defaultServiceManager());
-    if (sm != NULL) {
-        const String16 name("batterystats");
-        mBatteryStatService = interface_cast<IBatteryStats>(sm->getService(name));
-    }
+BatteryService::BatteryService() : mBatteryStatService(nullptr) {
 }
 
 bool BatteryService::addSensor(uid_t uid, int handle) {
@@ -61,7 +56,7 @@
 
 
 void BatteryService::enableSensorImpl(uid_t uid, int handle) {
-    if (mBatteryStatService != 0) {
+    if (checkService()) {
         if (addSensor(uid, handle)) {
             int64_t identity = IPCThreadState::self()->clearCallingIdentity();
             mBatteryStatService->noteStartSensor(uid, handle);
@@ -70,7 +65,7 @@
     }
 }
 void BatteryService::disableSensorImpl(uid_t uid, int handle) {
-    if (mBatteryStatService != 0) {
+    if (checkService()) {
         if (removeSensor(uid, handle)) {
             int64_t identity = IPCThreadState::self()->clearCallingIdentity();
             mBatteryStatService->noteStopSensor(uid, handle);
@@ -80,7 +75,7 @@
 }
 
 void BatteryService::cleanupImpl(uid_t uid) {
-    if (mBatteryStatService != 0) {
+    if (checkService()) {
         Mutex::Autolock _l(mActivationsLock);
         int64_t identity = IPCThreadState::self()->clearCallingIdentity();
         for (size_t i=0 ; i<mActivations.size() ; i++) {
@@ -95,6 +90,17 @@
     }
 }
 
+bool BatteryService::checkService() {
+    if (mBatteryStatService == nullptr) {
+        const sp<IServiceManager> sm(defaultServiceManager());
+        if (sm != NULL) {
+            const String16 name("batterystats");
+            mBatteryStatService = interface_cast<IBatteryStats>(sm->getService(name));
+        }
+    }
+    return mBatteryStatService != nullptr;
+}
+
 ANDROID_SINGLETON_STATIC_INSTANCE(BatteryService)
 
 // ---------------------------------------------------------------------------
diff --git a/services/sensorservice/BatteryService.h b/services/sensorservice/BatteryService.h
index 08ba857..43a750c 100644
--- a/services/sensorservice/BatteryService.h
+++ b/services/sensorservice/BatteryService.h
@@ -49,6 +49,7 @@
     SortedVector<Info> mActivations;
     bool addSensor(uid_t uid, int handle);
     bool removeSensor(uid_t uid, int handle);
+    bool checkService();
 
 public:
     static void enableSensor(uid_t uid, int handle) {
diff --git a/services/surfaceflinger/Android.mk b/services/surfaceflinger/Android.mk
index 7135ed3..4369fd2 100644
--- a/services/surfaceflinger/Android.mk
+++ b/services/surfaceflinger/Android.mk
@@ -52,7 +52,11 @@
     LOCAL_SRC_FILES += \
         SurfaceFlinger.cpp \
         DisplayHardware/HWComposer.cpp
+    ifeq ($(TARGET_USES_HWC2ON1ADAPTER), true)
+        LOCAL_CFLAGS += -DBYPASS_IHWC
+    endif
 else
+    LOCAL_CFLAGS += -DBYPASS_IHWC
     LOCAL_SRC_FILES += \
         SurfaceFlinger_hwc1.cpp \
         DisplayHardware/HWComposer_hwc1.cpp
@@ -135,7 +139,8 @@
     liblog \
     libdl \
     libhardware \
-    libhidl \
+    libhidlbase \
+    libhidltransport \
     libhwbinder \
     libutils \
     libEGL \
@@ -147,16 +152,14 @@
     libpowermanager \
     libvulkan \
     libprotobuf-cpp-lite \
-    libhidl \
-    libhwbinder \
     libbase \
-    libutils \
     android.hardware.power@1.0
 
 LOCAL_EXPORT_SHARED_LIBRARY_HEADERS := \
     android.hardware.graphics.allocator@2.0 \
     android.hardware.graphics.composer@2.1 \
-    libhidl \
+    libhidlbase \
+    libhidltransport \
     libhwbinder
 
 LOCAL_CFLAGS += -Wall -Werror -Wunused -Wunreachable-code
diff --git a/services/surfaceflinger/DisplayHardware/ComposerHal.cpp b/services/surfaceflinger/DisplayHardware/ComposerHal.cpp
index 128e2b0..06f67ba 100644
--- a/services/surfaceflinger/DisplayHardware/ComposerHal.cpp
+++ b/services/surfaceflinger/DisplayHardware/ComposerHal.cpp
@@ -26,6 +26,7 @@
 
 using hardware::Return;
 using hardware::hidl_vec;
+using hardware::hidl_handle;
 
 namespace Hwc2 {
 
@@ -39,14 +40,14 @@
         mHandle = (buffer) ? buffer : native_handle_init(mStorage, 0, 0);
     }
 
-    operator const native_handle_t*() const
+    operator const hidl_handle&() const
     {
         return mHandle;
     }
 
 private:
     NATIVE_HANDLE_DECLARE_STORAGE(mStorage, 0, 0);
-    const native_handle_t* mHandle;
+    hidl_handle mHandle;
 };
 
 class FenceHandle
@@ -55,13 +56,15 @@
     FenceHandle(int fd, bool owned)
         : mOwned(owned)
     {
+        native_handle_t* handle;
         if (fd >= 0) {
-            mHandle = native_handle_init(mStorage, 1, 0);
-            mHandle->data[0] = fd;
+            handle = native_handle_init(mStorage, 1, 0);
+            handle->data[0] = fd;
         } else {
             // nullptr is not a valid handle to HIDL
-            mHandle = native_handle_init(mStorage, 0, 0);
+            handle = native_handle_init(mStorage, 0, 0);
         }
+        mHandle = handle;
     }
 
     ~FenceHandle()
@@ -71,7 +74,7 @@
         }
     }
 
-    operator const native_handle_t*() const
+    operator const hidl_handle&() const
     {
         return mHandle;
     }
@@ -79,7 +82,7 @@
 private:
     bool mOwned;
     NATIVE_HANDLE_DECLARE_STORAGE(mStorage, 1, 0);
-    native_handle_t* mHandle;
+    hidl_handle mHandle;
 };
 
 // assume NO_RESOURCES when Status::isOk returns false
diff --git a/services/surfaceflinger/DisplayHardware/ComposerHal.h b/services/surfaceflinger/DisplayHardware/ComposerHal.h
index fd74a92..b8f7c20 100644
--- a/services/surfaceflinger/DisplayHardware/ComposerHal.h
+++ b/services/surfaceflinger/DisplayHardware/ComposerHal.h
@@ -27,7 +27,12 @@
 
 namespace Hwc2 {
 
-using android::hardware::graphics::allocator::V2_0::PixelFormat;
+using android::hardware::graphics::common::V1_0::ColorMode;
+using android::hardware::graphics::common::V1_0::ColorTransform;
+using android::hardware::graphics::common::V1_0::Dataspace;
+using android::hardware::graphics::common::V1_0::Hdr;
+using android::hardware::graphics::common::V1_0::PixelFormat;
+using android::hardware::graphics::common::V1_0::Transform;
 
 using android::hardware::graphics::composer::V2_1::IComposer;
 using android::hardware::graphics::composer::V2_1::IComposerCallback;
@@ -36,12 +41,6 @@
 using android::hardware::graphics::composer::V2_1::Layer;
 using android::hardware::graphics::composer::V2_1::Config;
 
-using android::hardware::graphics::composer::V2_1::ColorMode;
-using android::hardware::graphics::composer::V2_1::Hdr;
-using android::hardware::graphics::composer::V2_1::Dataspace;
-using android::hardware::graphics::composer::V2_1::ColorTransform;
-using android::hardware::graphics::composer::V2_1::Transform;
-
 // Composer is a wrapper to IComposer, a proxy to server-side composer.
 class Composer {
 public:
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp
index c79caf4..31af8a1 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp
@@ -614,6 +614,8 @@
     auto error = static_cast<Error>(intError);
 
     if (error != Error::None) {
+        ALOGE("Unable to get active config for mId:[%" PRIu64 "]", mId);
+        *outConfig = nullptr;
         return error;
     }
 
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.h b/services/surfaceflinger/DisplayHardware/HWC2.h
index 1145ba1..1c709b2 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.h
+++ b/services/surfaceflinger/DisplayHardware/HWC2.h
@@ -17,10 +17,6 @@
 #ifndef ANDROID_SF_HWC2_H
 #define ANDROID_SF_HWC2_H
 
-#ifndef USE_HWC2
-#define BYPASS_IHWC
-#endif
-
 #define HWC2_INCLUDE_STRINGIFICATION
 #define HWC2_USE_CPP11
 #include <hardware/hwcomposer2.h>
diff --git a/services/surfaceflinger/DisplayHardware/HWC2On1Adapter.cpp b/services/surfaceflinger/DisplayHardware/HWC2On1Adapter.cpp
index b699b2c..1727bd6 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2On1Adapter.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWC2On1Adapter.cpp
@@ -132,6 +132,7 @@
     mHwc1Device(hwc1Device),
     mHwc1MinorVersion(getMinorVersion(hwc1Device)),
     mHwc1SupportsVirtualDisplays(false),
+    mHwc1SupportsBackgroundColor(false),
     mHwc1Callbacks(std::make_unique<Callbacks>(*this)),
     mCapabilities(),
     mLayers(),
@@ -807,7 +808,10 @@
         return Error::None;
     }
 
-    *outDisplayRequests = mChanges->getDisplayRequests();
+    // Display requests (HWC2::DisplayRequest) are not supported by hwc1:
+    // A hwc1 has always zero requests for the client.
+    *outDisplayRequests = 0;
+
     uint32_t numWritten = 0;
     for (const auto& request : mChanges->getLayerRequests()) {
         if (numWritten == *outNumElements) {
@@ -1841,29 +1845,39 @@
 
     auto activeConfig = mDevice.mHwc1Device->getActiveConfig(
             mDevice.mHwc1Device, mHwc1Id);
-    if (activeConfig >= 0) {
-        for (const auto& config : mConfigs) {
-            if (config->hasHwc1Id(activeConfig)) {
-                ALOGV("Setting active config to %d for HWC1 config %u",
-                        config->getId(), activeConfig);
-                mActiveConfig = config;
-                if (config->getColorModeForHwc1Id(activeConfig, &mActiveColorMode) != Error::None) {
-                    // This should never happen since we checked for the config's presence before
-                    // setting it as active.
-                    ALOGE("Unable to find color mode for active HWC1 config %d",
-                            config->getId());
-                    mActiveColorMode = HAL_COLOR_MODE_NATIVE;
-                }
-                break;
+
+    // Some devices startup without an activeConfig:
+    // We need to set one ourselves.
+    if (activeConfig == HWC_ERROR) {
+        ALOGV("There is no active configuration: Picking the first one: 0.");
+        const int defaultIndex = 0;
+        mDevice.mHwc1Device->setActiveConfig(mDevice.mHwc1Device, mHwc1Id, defaultIndex);
+        activeConfig = defaultIndex;
+    }
+
+    for (const auto& config : mConfigs) {
+        if (config->hasHwc1Id(activeConfig)) {
+            ALOGE("Setting active config to %d for HWC1 config %u", config->getId(), activeConfig);
+            mActiveConfig = config;
+            if (config->getColorModeForHwc1Id(activeConfig, &mActiveColorMode) != Error::None) {
+                // This should never happen since we checked for the config's presence before
+                // setting it as active.
+                ALOGE("Unable to find color mode for active HWC1 config %d", config->getId());
+                mActiveColorMode = HAL_COLOR_MODE_NATIVE;
             }
-        }
-        if (!mActiveConfig) {
-            ALOGV("Unable to find active HWC1 config %u, defaulting to "
-                    "config 0", activeConfig);
-            mActiveConfig = mConfigs[0];
-            mActiveColorMode = HAL_COLOR_MODE_NATIVE;
+            break;
         }
     }
+    if (!mActiveConfig) {
+        ALOGV("Unable to find active HWC1 config %u, defaulting to "
+                "config 0", activeConfig);
+        mActiveConfig = mConfigs[0];
+        mActiveColorMode = HAL_COLOR_MODE_NATIVE;
+    }
+
+
+
+
 }
 
 void HWC2On1Adapter::Display::reallocateHwc1Contents()
@@ -2262,7 +2276,18 @@
         bool applyAllState)
 {
     if (applyAllState || mColor.isDirty()) {
-        hwc1Layer.backgroundColor = mColor.getPendingValue();
+        // If the device does not support background color it is likely to make
+        // assumption regarding backgroundColor and handle (both fields occupy
+        // the same location in hwc_layer_1_t union).
+        // To not confuse these devices we don't set background color and we
+        // make sure handle is a null pointer.
+        if (mDisplay.getDevice().supportsBackgroundColor()) {
+            hwc1Layer.backgroundColor = mColor.getPendingValue();
+            mHasUnsupportedBackgroundColor = false;
+        } else {
+            hwc1Layer.handle = nullptr;
+            mHasUnsupportedBackgroundColor = true;
+        }
         mColor.latch();
     }
 }
@@ -2289,7 +2314,7 @@
     // supports plane alpha (depending on the version). These require us to drop
     // some or all layers to client composition.
     if (mHasUnsupportedDataspace || mHasUnsupportedPlaneAlpha ||
-            mDisplay.hasColorTransform()) {
+            mDisplay.hasColorTransform() || mHasUnsupportedBackgroundColor) {
         hwc1Layer.compositionType = HWC_FRAMEBUFFER;
         hwc1Layer.flags = HWC_SKIP_LAYER;
         return;
@@ -2359,6 +2384,18 @@
     if (mHwc1MinorVersion >= 4U) {
         mCapabilities.insert(Capability::SidebandStream);
     }
+
+    // Check for HWC background color layer support.
+    if (mHwc1MinorVersion >= 1U) {
+        int backgroundColorSupported = 0;
+        auto result = mHwc1Device->query(mHwc1Device,
+                                         HWC_BACKGROUND_LAYER_SUPPORTED,
+                                         &backgroundColorSupported);
+        if ((result == 0) && (backgroundColorSupported == 1)) {
+            ALOGV("Found support for HWC background color");
+            mHwc1SupportsBackgroundColor = true;
+        }
+    }
 }
 
 HWC2On1Adapter::Display* HWC2On1Adapter::getDisplay(hwc2_display_t id)
diff --git a/services/surfaceflinger/DisplayHardware/HWC2On1Adapter.h b/services/surfaceflinger/DisplayHardware/HWC2On1Adapter.h
index 047e3aa..aa96004 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2On1Adapter.h
+++ b/services/surfaceflinger/DisplayHardware/HWC2On1Adapter.h
@@ -63,6 +63,10 @@
         getAdapter(device)->doGetCapabilities(outCount, outCapabilities);
     }
 
+    bool supportsBackgroundColor() {
+        return mHwc1SupportsBackgroundColor;
+    }
+
     // getFunction
 
     hwc2_function_pointer_t doGetFunction(HWC2::FunctionDescriptor descriptor);
@@ -134,8 +138,7 @@
     class DeferredFence {
         public:
             DeferredFence()
-              : mMutex(),
-                mFences({Fence::NO_FENCE, Fence::NO_FENCE}) {}
+              : mFences({Fence::NO_FENCE, Fence::NO_FENCE}) {}
 
             void add(int32_t fenceFd) {
                 mFences.emplace(new Fence(fenceFd));
@@ -147,7 +150,6 @@
             }
 
         private:
-            mutable std::mutex mMutex;
             std::queue<sp<Fence>> mFences;
     };
 
@@ -306,14 +308,6 @@
                         return mLayerRequests;
                     }
 
-                    int32_t getDisplayRequests() const {
-                        int32_t requests = 0;
-                        for (auto request : mDisplayRequests) {
-                            requests |= static_cast<int32_t>(request);
-                        }
-                        return requests;
-                    }
-
                     void addTypeChange(hwc2_layer_t layerId,
                             HWC2::Composition type) {
                         mTypeChanges.insert({layerId, type});
@@ -331,7 +325,6 @@
                             mTypeChanges;
                     std::unordered_map<hwc2_layer_t, HWC2::LayerRequest>
                             mLayerRequests;
-                    std::unordered_set<HWC2::DisplayRequest> mDisplayRequests;
             };
 
             std::shared_ptr<const Config>
@@ -576,6 +569,7 @@
             size_t mHwc1Id;
             bool mHasUnsupportedDataspace;
             bool mHasUnsupportedPlaneAlpha;
+            bool mHasUnsupportedBackgroundColor;
     };
 
     template <typename ...Args>
@@ -656,6 +650,7 @@
     struct hwc_composer_device_1* const mHwc1Device;
     const uint8_t mHwc1MinorVersion;
     bool mHwc1SupportsVirtualDisplays;
+    bool mHwc1SupportsBackgroundColor;
 
     class Callbacks;
     const std::unique_ptr<Callbacks> mHwc1Callbacks;
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index b4d698f..d590649 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -309,22 +309,22 @@
     return layer;
 }
 
-nsecs_t HWComposer::getRefreshTimestamp(int32_t disp) const {
+nsecs_t HWComposer::getRefreshTimestamp(int32_t displayId) const {
     // this returns the last refresh timestamp.
     // if the last one is not available, we estimate it based on
     // the refresh period and whatever closest timestamp we have.
     Mutex::Autolock _l(mLock);
     nsecs_t now = systemTime(CLOCK_MONOTONIC);
-    auto vsyncPeriod = getActiveConfig(disp)->getVsyncPeriod();
-    return now - ((now - mLastHwVSync[disp]) % vsyncPeriod);
+    auto vsyncPeriod = getActiveConfig(displayId)->getVsyncPeriod();
+    return now - ((now - mLastHwVSync[displayId]) % vsyncPeriod);
 }
 
-bool HWComposer::isConnected(int32_t disp) const {
-    if (!isValidDisplay(disp)) {
-        ALOGE("isConnected: Attempted to access invalid display %d", disp);
+bool HWComposer::isConnected(int32_t displayId) const {
+    if (!isValidDisplay(displayId)) {
+        ALOGE("isConnected: Attempted to access invalid display %d", displayId);
         return false;
     }
-    return mDisplayData[disp].hwcDisplay->isConnected();
+    return mDisplayData[displayId].hwcDisplay->isConnected();
 }
 
 std::vector<std::shared_ptr<const HWC2::Display::Config>>
@@ -346,14 +346,14 @@
 std::shared_ptr<const HWC2::Display::Config>
         HWComposer::getActiveConfig(int32_t displayId) const {
     if (!isValidDisplay(displayId)) {
-        ALOGE("getActiveConfigs: Attempted to access invalid display %d",
+        ALOGV("getActiveConfigs: Attempted to access invalid display %d",
                 displayId);
         return nullptr;
     }
     std::shared_ptr<const HWC2::Display::Config> config;
     auto error = mDisplayData[displayId].hwcDisplay->getActiveConfig(&config);
     if (error == HWC2::Error::BadConfig) {
-        ALOGV("getActiveConfig: No config active, returning null");
+        ALOGE("getActiveConfig: No config active, returning null");
         return nullptr;
     } else if (error != HWC2::Error::None) {
         ALOGE("getActiveConfig failed for display %d: %s (%d)", displayId,
@@ -408,14 +408,15 @@
 }
 
 
-void HWComposer::setVsyncEnabled(int32_t disp, HWC2::Vsync enabled) {
-    if (disp < 0 || disp >= HWC_DISPLAY_VIRTUAL) {
-        ALOGD("setVsyncEnabled: Ignoring for virtual display %d", disp);
+void HWComposer::setVsyncEnabled(int32_t displayId, HWC2::Vsync enabled) {
+    if (displayId < 0 || displayId >= HWC_DISPLAY_VIRTUAL) {
+        ALOGD("setVsyncEnabled: Ignoring for virtual display %d", displayId);
         return;
     }
 
-    if (!isValidDisplay(disp)) {
-        ALOGE("setVsyncEnabled: Attempted to access invalid display %d", disp);
+    if (!isValidDisplay(displayId)) {
+        ALOGE("setVsyncEnabled: Attempted to access invalid display %d",
+               displayId);
         return;
     }
 
@@ -424,7 +425,7 @@
     // that even if HWC blocks (which it shouldn't), it won't
     // affect other threads.
     Mutex::Autolock _l(mVsyncLock);
-    auto& displayData = mDisplayData[disp];
+    auto& displayData = mDisplayData[displayId];
     if (enabled != displayData.vsyncEnabled) {
         ATRACE_CALL();
         auto error = displayData.hwcDisplay->setVsyncEnabled(enabled);
@@ -432,12 +433,12 @@
             displayData.vsyncEnabled = enabled;
 
             char tag[16];
-            snprintf(tag, sizeof(tag), "HW_VSYNC_ON_%1u", disp);
+            snprintf(tag, sizeof(tag), "HW_VSYNC_ON_%1u", displayId);
             ATRACE_INT(tag, enabled == HWC2::Vsync::Enable ? 1 : 0);
         } else {
             ALOGE("setVsyncEnabled: Failed to set vsync to %s on %d/%" PRIu64
-                    ": %s (%d)", to_string(enabled).c_str(), disp,
-                    mDisplayData[disp].hwcDisplay->getId(),
+                    ": %s (%d)", to_string(enabled).c_str(), displayId,
+                    mDisplayData[displayId].hwcDisplay->getId(),
                     to_string(error).c_str(), static_cast<int32_t>(error));
         }
     }
@@ -619,7 +620,7 @@
     return displayFences[layer];
 }
 
-status_t HWComposer::commit(int32_t displayId) {
+status_t HWComposer::presentAndGetReleaseFences(int32_t displayId) {
     ATRACE_CALL();
 
     if (!isValidDisplay(displayId)) {
@@ -630,15 +631,16 @@
     auto& hwcDisplay = displayData.hwcDisplay;
     auto error = hwcDisplay->present(&displayData.lastRetireFence);
     if (error != HWC2::Error::None) {
-        ALOGE("commit: present failed for display %d: %s (%d)", displayId,
-                to_string(error).c_str(), static_cast<int32_t>(error));
+        ALOGE("presentAndGetReleaseFences: failed for display %d: %s (%d)",
+              displayId, to_string(error).c_str(), static_cast<int32_t>(error));
         return UNKNOWN_ERROR;
     }
 
     std::unordered_map<std::shared_ptr<HWC2::Layer>, sp<Fence>> releaseFences;
     error = hwcDisplay->getReleaseFences(&releaseFences);
     if (error != HWC2::Error::None) {
-        ALOGE("commit: Failed to get release fences for display %d: %s (%d)",
+        ALOGE("presentAndGetReleaseFences: Failed to get release fences "
+              "for display %d: %s (%d)",
                 displayId, to_string(error).c_str(),
                 static_cast<int32_t>(error));
         return UNKNOWN_ERROR;
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index 64c0d1b..e6f1b25 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -97,8 +97,8 @@
     status_t setClientTarget(int32_t displayId, const sp<Fence>& acquireFence,
             const sp<GraphicBuffer>& target, android_dataspace_t dataspace);
 
-    // Finalize the layers and present them
-    status_t commit(int32_t displayId);
+    // Present layers to the display and read releaseFences.
+    status_t presentAndGetReleaseFences(int32_t displayId);
 
     // set power mode
     status_t setPowerMode(int32_t displayId, int mode);
@@ -145,12 +145,12 @@
 
     // Events handling ---------------------------------------------------------
 
-    void setVsyncEnabled(int32_t disp, HWC2::Vsync enabled);
+    void setVsyncEnabled(int32_t displayId, HWC2::Vsync enabled);
 
     // Query display parameters.  Pass in a display index (e.g.
     // HWC_DISPLAY_PRIMARY).
-    nsecs_t getRefreshTimestamp(int32_t disp) const;
-    bool isConnected(int32_t disp) const;
+    nsecs_t getRefreshTimestamp(int32_t displayId) const;
+    bool isConnected(int32_t displayId) const;
 
     // Non-const because it can update configMap inside of mDisplayData
     std::vector<std::shared_ptr<const HWC2::Display::Config>>
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 2f273ae..fa889ee 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -698,6 +698,10 @@
 }
 
 int SurfaceFlinger::getActiveConfig(const sp<IBinder>& display) {
+    if (display == NULL) {
+        ALOGE("%s : display is NULL", __func__);
+        return BAD_VALUE;
+    }
     sp<DisplayDevice> device(getDisplayDevice(display));
     if (device != NULL) {
         return device->getActiveConfig();
@@ -1485,7 +1489,7 @@
         }
         const auto hwcId = displayDevice->getHwcDisplayId();
         if (hwcId >= 0) {
-            mHwc->commit(hwcId);
+            mHwc->presentAndGetReleaseFences(hwcId);
         }
         displayDevice->onSwapBuffersCompleted();
         displayDevice->makeCurrent(mEGLDisplay, mEGLContext);