Merge "recovery: report compliant reboot reason (Part Deux)"
diff --git a/bootstat/bootstat.cpp b/bootstat/bootstat.cpp
index 872188b..c7be00b 100644
--- a/bootstat/bootstat.cpp
+++ b/bootstat/bootstat.cpp
@@ -1092,17 +1092,8 @@
 void LogBootInfoToStatsd(std::chrono::milliseconds end_time,
                          std::chrono::milliseconds total_duration, int32_t bootloader_duration_ms,
                          double time_since_last_boot_sec) {
-  const auto reason = android::base::GetProperty(bootloader_reboot_reason_property, "");
-
-  if (reason.empty()) {
-    android::util::stats_write(android::util::BOOT_SEQUENCE_REPORTED, "<EMPTY>", "<EMPTY>",
-                               end_time.count(), total_duration.count(),
-                               (int64_t)bootloader_duration_ms,
-                               (int64_t)time_since_last_boot_sec * 1000);
-    return;
-  }
-
-  const auto system_reason = android::base::GetProperty(system_reboot_reason_property, "");
+  const auto reason = android::base::GetProperty(bootloader_reboot_reason_property, "<EMPTY>");
+  const auto system_reason = android::base::GetProperty(system_reboot_reason_property, "<EMPTY>");
   android::util::stats_write(android::util::BOOT_SEQUENCE_REPORTED, reason.c_str(),
                              system_reason.c_str(), end_time.count(), total_duration.count(),
                              (int64_t)bootloader_duration_ms,
diff --git a/fs_mgr/tests/adb-remount-test.sh b/fs_mgr/tests/adb-remount-test.sh
index c22176b..04422f9 100755
--- a/fs_mgr/tests/adb-remount-test.sh
+++ b/fs_mgr/tests/adb-remount-test.sh
@@ -219,13 +219,31 @@
   echo ${hours}:`expr ${minutes} / 10``expr ${minutes} % 10`:`expr ${seconds} / 10``expr ${seconds} % 10`
 }
 
+[ "USAGE: USB_DEVICE=\`usb_devnum [--next]\`
+
+USB_DEVICE contains cache. Update if system changes.
+
+Returns: the devnum for the USB_SERIAL device" ]
+usb_devnum() {
+  if [ -n "${USB_SERIAL}" ]; then
+    local usb_device=`cat ${USB_SERIAL%/serial}/devnum 2>/dev/null | tr -d ' \t\r\n'`
+    if [ -n "${usb_device}" ]; then
+      USB_DEVICE=dev${usb_device}
+    elif [ -n "${USB_DEVICE}" -a "${1}" ]; then
+      USB_DEVICE=dev`expr ${USB_DEVICE#dev} + 1`
+    fi
+    echo "${USB_DEVICE}"
+  fi
+}
+
 [ "USAGE: adb_wait [timeout]
 
 Returns: waits until the device has returned for adb or optional timeout" ]
 adb_wait() {
   local ret
   if [ -n "${1}" ]; then
-    echo -n ". . . waiting `format_duration ${1}`" ${ANDROID_SERIAL} ${USB_ADDRESS} "${CR}"
+    USB_DEVICE=`usb_devnum --next`
+    echo -n ". . . waiting `format_duration ${1}`" ${ANDROID_SERIAL} ${USB_ADDRESS} ${USB_DEVICE} "${CR}"
     timeout --preserve-status --signal=KILL ${1} adb wait-for-device 2>/dev/null
     ret=${?}
     echo -n "                                                                             ${CR}"
@@ -233,6 +251,7 @@
     adb wait-for-device
     ret=${?}
   fi
+  USB_DEVICE=`usb_devnum`
   if [ 0 = ${ret} -a -n "${ACTIVE_SLOT}" ]; then
     local active_slot=`get_active_slot`
     if [ X"${ACTIVE_SLOT}" != X"${active_slot}" ]; then
@@ -242,12 +261,15 @@
   return ${ret}
 }
 
-[ "USAGE: usb_status > stdout
+[ "USAGE: usb_status > stdout 2> stderr
 
-If adb_wait failed, check if device is in adb, recovery or fastboot mode
-and report status string.
+Assumes referenced right after adb_wait or fastboot_wait failued.
+If wait failed, check if device is in adb, recovery or fastboot mode
+and report status strings like  \"(USB stack borken?)\",
+\"(In fastboot mode)\", \"(In recovery mode)\" or \"(in adb mode)\".
+Additional diagnostics may be provided to the stderr output.
 
-Returns: \"(USB stack borken?)\", \"(In fastboot mode)\" or \"(in adb mode)\"" ]
+Returns: USB status string" ]
 usb_status() {
   if inFastboot; then
     echo "(In fastboot mode)"
@@ -256,7 +278,20 @@
   elif inAdb; then
     echo "(In adb mode)"
   else
-    echo "(USB stack borken?)"
+    echo "(USB stack borken for ${USB_ADDRESS})"
+    USB_DEVICE=`usb_devnum`
+    if [ -n "${USB_DEVICE}" ]; then
+      echo "# lsusb -v -s ${USB_DEVICE#dev}"
+      local D=`lsusb -v -s ${USB_DEVICE#dev} 2>&1`
+      if [ -n "${D}" ]; then
+        echo "${D}"
+      else
+        lsusb -v
+      fi
+    else
+      echo "# lsusb -v (expected device missing)"
+      lsusb -v
+    fi >&2
   fi
 }
 
@@ -268,7 +303,8 @@
   # fastboot has no wait-for-device, but it does an automatic
   # wait and requires (even a nonsensical) command to do so.
   if [ -n "${1}" ]; then
-    echo -n ". . . waiting `format_duration ${1}`" ${ANDROID_SERIAL} ${USB_ADDRESS} "${CR}"
+    USB_DEVICE=`usb_devnum --next`
+    echo -n ". . . waiting `format_duration ${1}`" ${ANDROID_SERIAL} ${USB_ADDRESS} ${USB_DEVICE} "${CR}"
     timeout --preserve-status --signal=KILL ${1} fastboot wait-for-device >/dev/null 2>/dev/null
     ret=${?}
     echo -n "                                                                             ${CR}"
@@ -278,11 +314,12 @@
   fi ||
     inFastboot
   ret=${?}
+  USB_DEVICE=`usb_devnum`
   if [ 0 = ${ret} -a -n "${ACTIVE_SLOT}" ]; then
     local active_slot=`get_active_slot`
     if [ X"${ACTIVE_SLOT}" != X"${active_slot}" ]; then
-      echo "${ORANGE}[  WARNING ]${NORMAL} Active slot changed from ${ACTIVE_SLOT} to ${active_slot}" >&2
-    fi
+      echo "${ORANGE}[  WARNING ]${NORMAL} Active slot changed from ${ACTIVE_SLOT} to ${active_slot}"
+    fi >&2
   fi
   return ${ret}
 }
@@ -293,7 +330,8 @@
 recovery_wait() {
   local ret
   if [ -n "${1}" ]; then
-    echo -n ". . . waiting `format_duration ${1}`" ${ANDROID_SERIAL} ${USB_ADDRESS} "${CR}"
+    USB_DEVICE=`usb_devnum --next`
+    echo -n ". . . waiting `format_duration ${1}`" ${ANDROID_SERIAL} ${USB_ADDRESS} ${USB_DEVICE} "${CR}"
     timeout --preserve-status --signal=KILL ${1} adb wait-for-recovery 2>/dev/null
     ret=${?}
     echo -n "                                                                             ${CR}"
@@ -301,11 +339,12 @@
     adb wait-for-recovery
     ret=${?}
   fi
+  USB_DEVICE=`usb_devnum`
   if [ 0 = ${ret} -a -n "${ACTIVE_SLOT}" ]; then
     local active_slot=`get_active_slot`
     if [ X"${ACTIVE_SLOT}" != X"${active_slot}" ]; then
-      echo "${ORANGE}[  WARNING ]${NORMAL} Active slot changed from ${ACTIVE_SLOT} to ${active_slot}" >&2
-    fi
+      echo "${ORANGE}[  WARNING ]${NORMAL} Active slot changed from ${ACTIVE_SLOT} to ${active_slot}"
+    fi >&2
   fi
   return ${ret}
 }
@@ -370,10 +409,10 @@
     O="${1}: <empty>"
   fi
   if [ -n "${2}" -a "${1}: ${2}" != "${O}" ]; then
-    echo "${2} != ${O}" >&2
+    echo "${2} != ${O}"
     false
     return
-  fi
+  fi >&2
   echo ${O} >&2
 }
 
@@ -430,16 +469,16 @@
 Returns: exit failure, report status" ]
 die() {
   if [ X"-d" = X"${1}" ]; then
-    adb_logcat -b all -v nsec -d >&2
+    adb_logcat -b all -v nsec -d
     shift
   elif [ X"-t" = X"${1}" ]; then
     if [ -n "${2}" ]; then
-      adb_logcat -b all -v nsec -t ${2} >&2
+      adb_logcat -b all -v nsec -t ${2}
     else
-      adb_logcat -b all -v nsec -d >&2
+      adb_logcat -b all -v nsec -d
     fi
     shift 2
-  fi
+  fi >&2
   echo "${RED}[  FAILED  ]${NORMAL} ${@}" >&2
   cleanup
   restore
@@ -464,39 +503,39 @@
   if ! ( echo X"${rval}" | grep '^X'"${lval}"'$' >/dev/null 2>/dev/null ); then
     if [ `echo ${lval}${rval}${*} | wc -c` -gt 50 -o "${rval}" != "${rval%
 *}" ]; then
-      echo "${prefix} expected \"${lval}\"" >&2
+      echo "${prefix} expected \"${lval}\""
       echo "${prefix} got \"${rval}\"" |
         sed ': again
              N
              s/\(\n\)\([^ ]\)/\1             \2/
-             t again' >&2
+             t again'
       if [ -n "${*}" ] ; then
-        echo "${prefix} ${*}" >&2
+        echo "${prefix} ${*}"
       fi
     else
-      echo "${prefix} expected \"${lval}\" got \"${rval}\" ${*}" >&2
-    fi
+      echo "${prefix} expected \"${lval}\" got \"${rval}\" ${*}"
+    fi >&2
     return ${error}
   fi
   if [ -n "${*}" ] ; then
     prefix="${GREEN}[     INFO ]${NORMAL}"
     if [ X"${lval}" != X"${rval}" ]; then  # we were supplied a regex?
       if [ `echo ${lval}${rval}${*} | wc -c` -gt 60 -o "${rval}" != "${rval% *}" ]; then
-        echo "${prefix} ok \"${lval}\"" >&2
+        echo "${prefix} ok \"${lval}\""
         echo "       = \"${rval}\"" |
           sed ': again
                N
                s/\(\n\)\([^ ]\)/\1          \2/
-               t again' >&2
+               t again'
         if [ -n "${*}" ] ; then
-          echo "${prefix} ${*}" >&2
+          echo "${prefix} ${*}"
         fi
       else
-        echo "${prefix} ok \"${lval}\" = \"${rval}\" ${*}" >&2
+        echo "${prefix} ok \"${lval}\" = \"${rval}\" ${*}"
       fi
     else
-      echo "${prefix} ok \"${lval}\" ${*}" >&2
-    fi
+      echo "${prefix} ok \"${lval}\" ${*}"
+    fi >&2
   fi
   return 0
 }
@@ -631,7 +670,8 @@
   USB_ADDRESS=usb${USB_ADDRESS##*/}
 fi
 [ -z "${ANDROID_SERIAL}${USB_ADDRESS}" ] ||
-  echo "${BLUE}[     INFO ]${NORMAL}" ${ANDROID_SERIAL} ${USB_ADDRESS} >&2
+  USB_DEVICE=`usb_devnum`
+  echo "${BLUE}[     INFO ]${NORMAL}" ${ANDROID_SERIAL} ${USB_ADDRESS} ${USB_DEVICE} >&2
 BUILD_DESCRIPTION=`get_property ro.build.description`
 [ -z "${BUILD_DESCRIPTION}" ] ||
   echo "${BLUE}[     INFO ]${NORMAL} ${BUILD_DESCRIPTION}" >&2
@@ -961,8 +1001,16 @@
 
 echo "${GREEN}[ RUN      ]${NORMAL} reboot to confirm content persistent" >&2
 
+fixup_from_recovery() {
+  inRecovery || return 1
+  echo "${ORANGE}[    ERROR ]${NORMAL} Device in recovery" >&2
+  adb reboot
+  adb_wait 2m
+}
+
 adb_reboot &&
   adb_wait 2m ||
+  fixup_from_recovery ||
   die "reboot after override content added failed `usb_status`"
 
 if ${overlayfs_needed}; then
@@ -1070,6 +1118,7 @@
     die "can not reboot out of fastboot"
   echo "${ORANGE}[  WARNING ]${NORMAL} adb after fastboot"
   adb_wait 2m ||
+    fixup_from_recovery ||
     die "did not reboot after flash `usb_status`"
   if ${overlayfs_needed}; then
     adb_root &&
@@ -1191,9 +1240,25 @@
 
 echo "${GREEN}[ RUN      ]${NORMAL} test raw remount commands" >&2
 
+fixup_from_fastboot() {
+  inFastboot || return 1
+  if [ -n "${ACTIVE_SLOT}" ]; then
+    local active_slot=`get_active_slot`
+    if [ X"${ACTIVE_SLOT}" != X"${active_slot}" ]; then
+      echo "${ORANGE}[    ERROR ]${NORMAL} Active slot changed from ${ACTIVE_SLOT} to ${active_slot}"
+    else
+      echo "${ORANGE}[    ERROR ]${NORMAL} Active slot to be set to ${ACTIVE_SLOT}"
+    fi >&2
+    fastboot --set-active=${ACTIVE_SLOT}
+  fi
+  fastboot reboot
+  adb_wait 2m
+}
+
 # Prerequisite is a prepped device from above.
 adb_reboot &&
   adb_wait 2m ||
+  fixup_from_fastboot ||
   die "lost device after reboot to ro state `usb_status`"
 adb_sh grep " /vendor .* rw," /proc/mounts >/dev/null </dev/null &&
   die "/vendor is not read-only"
@@ -1206,7 +1271,8 @@
 # Prerequisite is a prepped device from above.
 adb_reboot &&
   adb_wait 2m ||
-  die "lost device after reboot to ro state (USB stack broken?)"
+  fixup_from_fastboot ||
+  die "lost device after reboot to ro state `usb_status`"
 adb_sh grep " /vendor .* rw," /proc/mounts >/dev/null </dev/null &&
   die "/vendor is not read-only"
 adb_su remount vendor </dev/null ||
@@ -1226,7 +1292,8 @@
 done
 adb_reboot &&
   adb_wait 2m ||
-  die "lost device after reboot after wipe (USB stack broken?)"
+  fixup_from_fastboot ||
+  die "lost device after reboot after wipe `usb_status`"
 adb_sh grep " /vendor .* rw," /proc/mounts >/dev/null </dev/null &&
   die "/vendor is not read-only"
 adb_su remount vendor </dev/null ||
diff --git a/init/README.md b/init/README.md
index 806bfa7..6868378 100644
--- a/init/README.md
+++ b/init/README.md
@@ -35,24 +35,14 @@
 at the beginning of its execution.  It is responsible for the initial
 set up of the system.
 
-Devices that mount /system, /vendor through the first stage mount mechanism
-load all of the files contained within the
+Init loads all of the files contained within the
 /{system,vendor,odm}/etc/init/ directories immediately after loading
 the primary /init.rc.  This is explained in more details in the
 Imports section of this file.
 
-Legacy devices without the first stage mount mechanism do the following:
-1. /init.rc imports /init.${ro.hardware}.rc which is the primary
-   vendor supplied .rc file.
-2. During the mount\_all command, the init executable loads all of the
-   files contained within the /{system,vendor,odm}/etc/init/ directories.
-   These directories are intended for all Actions and Services used after
-   file system mounting.
-
-One may specify paths in the mount\_all command line to have it import
-.rc files at the specified paths instead of the default ones listed above.
-This is primarily for supporting factory mode and other non-standard boot
-modes.  The three default paths should be used for the normal boot process.
+Legacy devices without the first stage mount mechanism previously were
+able to import init scripts during mount_all, however that is deprecated
+and not allowed for devices launching after Q.
 
 The intention of these directories is:
 
@@ -88,14 +78,6 @@
 conflict resolution when multiple services are added to the system, as
 each one will go into a separate file.
 
-There are two options "early" and "late" in mount\_all command
-which can be set after optional paths. With "--early" set, the
-init executable will skip mounting entries with "latemount" flag
-and triggering fs encryption state event. With "--late" set,
-init executable will only mount entries with "latemount" flag but skip
-importing rc files. By default, no option is set, and mount\_all will
-process all entries in the given fstab.
-
 Actions
 -------
 Actions are named sequences of commands.  Actions have a trigger which
@@ -514,10 +496,12 @@
   will be updated if the directory exists already.
 
 `mount_all <fstab> [ <path> ]\* [--<option>]`
-> Calls fs\_mgr\_mount\_all on the given fs\_mgr-format fstab and imports .rc files
-  at the specified paths (e.g., on the partitions just mounted) with optional
+> Calls fs\_mgr\_mount\_all on the given fs\_mgr-format fstab with optional
   options "early" and "late".
-  Refer to the section of "Init .rc Files" for detail.
+  With "--early" set, the init executable will skip mounting entries with
+  "latemount" flag and triggering fs encryption state event. With "--late" set,
+  init executable will only mount entries with "latemount" flag. By default,
+  no option is set, and mount\_all will process all entries in the given fstab.
 
 `mount <type> <device> <dir> [ <flag>\* ] [<options>]`
 > Attempt to mount the named device at the directory _dir_
@@ -638,8 +622,9 @@
       `ro.boot.init_rc` during initial boot.
    2. When it imports /{system,vendor,odm}/etc/init/ for first stage mount
       devices immediately after importing /init.rc.
-   3. When it imports /{system,vendor,odm}/etc/init/ or .rc files at specified
-      paths during mount_all.
+   3. (Deprecated) When it imports /{system,vendor,odm}/etc/init/ or .rc files
+      at specified paths during mount_all, not allowed for devices launching
+      after Q.
 
 The order that files are imported is a bit complex for legacy reasons
 and to keep backwards compatibility.  It is not strictly guaranteed.
diff --git a/init/action.cpp b/init/action.cpp
index 94ccef2..a4f6936 100644
--- a/init/action.cpp
+++ b/init/action.cpp
@@ -127,7 +127,7 @@
     // report such failures unless we're running at the DEBUG log level.
     bool report_failure = !result.has_value();
     if (report_failure && android::base::GetMinimumLogSeverity() > android::base::DEBUG &&
-        result.error_errno() == ENOENT) {
+        result.error().as_errno == ENOENT) {
         report_failure = false;
     }
 
@@ -139,7 +139,7 @@
 
         LOG(INFO) << "Command '" << cmd_str << "' action=" << trigger_name << " (" << filename_
                   << ":" << command.line() << ") took " << duration.count() << "ms and "
-                  << (result ? "succeeded" : "failed: " + result.error_string());
+                  << (result ? "succeeded" : "failed: " + result.error().as_string);
     }
 }
 
diff --git a/init/builtins.cpp b/init/builtins.cpp
index 6ce7736..840f2d4 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -16,6 +16,7 @@
 
 #include "builtins.h"
 
+#include <android/api-level.h>
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -62,6 +63,7 @@
 
 #include "action_manager.h"
 #include "bootchart.h"
+#include "host_init_stubs.h"
 #include "init.h"
 #include "mount_namespace.h"
 #include "parser.h"
@@ -588,7 +590,7 @@
     auto mount_fstab_return_code = fs_mgr_mount_all(&fstab, mount_mode);
     property_set(prop_name, std::to_string(t.duration().count()));
 
-    if (import_rc) {
+    if (import_rc && SelinuxGetVendorAndroidVersion() <= __ANDROID_API_Q__) {
         /* Paths of .rc files are specified at the 2nd argument and beyond */
         import_late(args.args, 2, path_arg_end);
     }
diff --git a/init/firmware_handler.cpp b/init/firmware_handler.cpp
index 740e82c..c067f6f 100644
--- a/init/firmware_handler.cpp
+++ b/init/firmware_handler.cpp
@@ -80,15 +80,26 @@
         return;
     }
 
+    std::vector<std::string> attempted_paths_and_errors;
+
 try_loading_again:
+    attempted_paths_and_errors.clear();
     for (const auto& firmware_directory : firmware_directories_) {
         std::string file = firmware_directory + uevent.firmware;
         unique_fd fw_fd(open(file.c_str(), O_RDONLY | O_CLOEXEC));
-        struct stat sb;
-        if (fw_fd != -1 && fstat(fw_fd, &sb) != -1) {
-            LoadFirmware(uevent, root, fw_fd, sb.st_size, loading_fd, data_fd);
-            return;
+        if (fw_fd == -1) {
+            attempted_paths_and_errors.emplace_back("firmware: attempted " + file +
+                                                    ", open failed: " + strerror(errno));
+            continue;
         }
+        struct stat sb;
+        if (fstat(fw_fd, &sb) == -1) {
+            attempted_paths_and_errors.emplace_back("firmware: attempted " + file +
+                                                    ", fstat failed: " + strerror(errno));
+            continue;
+        }
+        LoadFirmware(uevent, root, fw_fd, sb.st_size, loading_fd, data_fd);
+        return;
     }
 
     if (booting) {
@@ -100,6 +111,9 @@
     }
 
     LOG(ERROR) << "firmware: could not find firmware for " << uevent.firmware;
+    for (const auto& message : attempted_paths_and_errors) {
+        LOG(ERROR) << message;
+    }
 
     // Write "-1" as our response to the kernel's firmware request, since we have nothing for it.
     write(loading_fd, "-1", 2);
diff --git a/init/init.cpp b/init/init.cpp
index 1f3c2fc..0615d44 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -280,13 +280,13 @@
     return control_message_functions;
 }
 
-void HandleControlMessage(const std::string& msg, const std::string& name, pid_t pid) {
+bool HandleControlMessage(const std::string& msg, const std::string& name, pid_t pid) {
     const auto& map = get_control_message_map();
     const auto it = map.find(msg);
 
     if (it == map.end()) {
         LOG(ERROR) << "Unknown control msg '" << msg << "'";
-        return;
+        return false;
     }
 
     std::string cmdline_path = StringPrintf("proc/%d/cmdline", pid);
@@ -315,17 +315,19 @@
         default:
             LOG(ERROR) << "Invalid function target from static map key '" << msg << "': "
                        << static_cast<std::underlying_type<ControlTarget>::type>(function.target);
-            return;
+            return false;
     }
 
     if (svc == nullptr) {
         LOG(ERROR) << "Could not find '" << name << "' for ctl." << msg;
-        return;
+        return false;
     }
 
     if (auto result = function.action(svc); !result) {
         LOG(ERROR) << "Could not ctl." << msg << " for '" << name << "': " << result.error();
+        return false;
     }
+    return true;
 }
 
 static Result<Success> wait_for_coldboot_done_action(const BuiltinArguments& args) {
diff --git a/init/init.h b/init/init.h
index a76da20..90ead0e 100644
--- a/init/init.h
+++ b/init/init.h
@@ -40,7 +40,7 @@
 Parser CreateParser(ActionManager& action_manager, ServiceList& service_list);
 Parser CreateServiceOnlyParser(ServiceList& service_list);
 
-void HandleControlMessage(const std::string& msg, const std::string& arg, pid_t pid);
+bool HandleControlMessage(const std::string& msg, const std::string& arg, pid_t pid);
 
 void property_changed(const std::string& name, const std::string& value);
 
diff --git a/init/keychords.cpp b/init/keychords.cpp
index f5ac44f..d0ca3e7 100644
--- a/init/keychords.cpp
+++ b/init/keychords.cpp
@@ -41,7 +41,7 @@
 
 Keychords::~Keychords() noexcept {
     if (inotify_fd_ >= 0) {
-        epoll_->UnregisterHandler(inotify_fd_).IgnoreError();
+        epoll_->UnregisterHandler(inotify_fd_);
         ::close(inotify_fd_);
     }
     while (!registration_.empty()) GeteventCloseDevice(registration_.begin()->first);
@@ -212,7 +212,7 @@
     auto it = registration_.find(device);
     if (it == registration_.end()) return;
     auto fd = (*it).second;
-    epoll_->UnregisterHandler(fd).IgnoreError();
+    epoll_->UnregisterHandler(fd);
     registration_.erase(it);
     ::close(fd);
 }
diff --git a/init/keychords_test.cpp b/init/keychords_test.cpp
index e5a6fd3..a3baeb1 100644
--- a/init/keychords_test.cpp
+++ b/init/keychords_test.cpp
@@ -213,7 +213,7 @@
 }
 
 void TestFrame::RelaxForMs(std::chrono::milliseconds wait) {
-    epoll_.Wait(wait).IgnoreError();
+    epoll_.Wait(wait);
 }
 
 void TestFrame::SetChord(int key, bool value) {
diff --git a/init/mount_handler.cpp b/init/mount_handler.cpp
index c8f0e76..b0b63c5 100644
--- a/init/mount_handler.cpp
+++ b/init/mount_handler.cpp
@@ -121,7 +121,7 @@
 }
 
 MountHandler::~MountHandler() {
-    if (fp_) epoll_->UnregisterHandler(fileno(fp_.get())).IgnoreError();
+    if (fp_) epoll_->UnregisterHandler(fileno(fp_.get()));
 }
 
 void MountHandler::MountHandlerFunction() {
diff --git a/init/property_service.cpp b/init/property_service.cpp
index a1e9551..ab5dd61 100644
--- a/init/property_service.cpp
+++ b/init/property_service.cpp
@@ -473,8 +473,9 @@
     }
 
     if (StartsWith(name, "ctl.")) {
-        HandleControlMessage(name.c_str() + 4, value, cr.pid);
-        return PROP_SUCCESS;
+        return HandleControlMessage(name.c_str() + 4, value, cr.pid)
+                       ? PROP_SUCCESS
+                       : PROP_ERROR_HANDLE_CONTROL_MESSAGE;
     }
 
     // sys.powerctl is a special property that is used to make the device reboot.  We want to log
@@ -885,8 +886,12 @@
     load_properties_from_file("/system/build.prop", nullptr, &properties);
     load_properties_from_file("/vendor/default.prop", nullptr, &properties);
     load_properties_from_file("/vendor/build.prop", nullptr, &properties);
-    load_properties_from_file("/odm/default.prop", nullptr, &properties);
-    load_properties_from_file("/odm/build.prop", nullptr, &properties);
+    if (SelinuxGetVendorAndroidVersion() >= __ANDROID_API_Q__) {
+        load_properties_from_file("/odm/etc/build.prop", nullptr, &properties);
+    } else {
+        load_properties_from_file("/odm/default.prop", nullptr, &properties);
+        load_properties_from_file("/odm/build.prop", nullptr, &properties);
+    }
     load_properties_from_file("/product/build.prop", nullptr, &properties);
     load_properties_from_file("/product_services/build.prop", nullptr, &properties);
     load_properties_from_file("/factory/factory.prop", "ro.*", &properties);
diff --git a/init/reboot.cpp b/init/reboot.cpp
index 0966b6c..54f68bb 100644
--- a/init/reboot.cpp
+++ b/init/reboot.cpp
@@ -527,13 +527,13 @@
 
             // start all animation classes if stopped.
             if (do_shutdown_animation) {
-                service->Start().IgnoreError();
+                service->Start();
             }
             service->SetShutdownCritical();  // will not check animation class separately
         }
 
         if (do_shutdown_animation) {
-            bootAnim->Start().IgnoreError();
+            bootAnim->Start();
             surfaceFlinger->SetShutdownCritical();
             bootAnim->SetShutdownCritical();
         }
diff --git a/init/result.h b/init/result.h
index 0e3fd3d..baa680d 100644
--- a/init/result.h
+++ b/init/result.h
@@ -68,14 +68,14 @@
 // if (!output) return Error() << "CalculateResult failed: " << output.error();
 // UseOutput(*output);
 
-#ifndef _INIT_RESULT_H
-#define _INIT_RESULT_H
+#pragma once
 
 #include <errno.h>
 
 #include <sstream>
 #include <string>
-#include <variant>
+
+#include <android-base/expected.h>
 
 namespace android {
 namespace init {
@@ -83,19 +83,24 @@
 struct ResultError {
     template <typename T>
     ResultError(T&& error_string, int error_errno)
-        : error_string(std::forward<T>(error_string)), error_errno(error_errno) {}
+        : as_string(std::forward<T>(error_string)), as_errno(error_errno) {}
 
-    std::string error_string;
-    int error_errno;
+    template <typename T>
+    operator android::base::expected<T, ResultError>() {
+        return android::base::unexpected(ResultError(as_string, as_errno));
+    }
+
+    std::string as_string;
+    int as_errno;
 };
 
 inline std::ostream& operator<<(std::ostream& os, const ResultError& t) {
-    os << t.error_string;
+    os << t.as_string;
     return os;
 }
 
 inline std::ostream& operator<<(std::ostream& os, ResultError&& t) {
-    os << std::move(t.error_string);
+    os << std::move(t.as_string);
     return os;
 }
 
@@ -105,20 +110,25 @@
     Error(int errno_to_append) : errno_(errno_to_append), append_errno_(true) {}
 
     template <typename T>
+    operator android::base::expected<T, ResultError>() {
+        return android::base::unexpected(ResultError(str(), errno_));
+    }
+
+    template <typename T>
     Error&& operator<<(T&& t) {
         ss_ << std::forward<T>(t);
         return std::move(*this);
     }
 
     Error&& operator<<(const ResultError& result_error) {
-        ss_ << result_error.error_string;
-        errno_ = result_error.error_errno;
+        ss_ << result_error.as_string;
+        errno_ = result_error.as_errno;
         return std::move(*this);
     }
 
     Error&& operator<<(ResultError&& result_error) {
-        ss_ << std::move(result_error.error_string);
-        errno_ = result_error.error_errno;
+        ss_ << std::move(result_error.as_string);
+        errno_ = result_error.as_errno;
         return std::move(*this);
     }
 
@@ -151,63 +161,10 @@
 }
 
 template <typename T>
-class [[nodiscard]] Result {
-  public:
-    Result() {}
-
-    template <typename U, typename... V,
-              typename = std::enable_if_t<!(std::is_same_v<std::decay_t<U>, Result<T>> &&
-                                            sizeof...(V) == 0)>>
-    Result(U&& result, V&&... results)
-        : contents_(std::in_place_index_t<0>(), std::forward<U>(result),
-                    std::forward<V>(results)...) {}
-
-    Result(Error&& error) : contents_(std::in_place_index_t<1>(), error.str(), error.get_errno()) {}
-    Result(const ResultError& result_error)
-        : contents_(std::in_place_index_t<1>(), result_error.error_string,
-                    result_error.error_errno) {}
-    Result(ResultError&& result_error)
-        : contents_(std::in_place_index_t<1>(), std::move(result_error.error_string),
-                    result_error.error_errno) {}
-
-    void IgnoreError() const {}
-
-    bool has_value() const { return contents_.index() == 0; }
-
-    T& value() & { return std::get<0>(contents_); }
-    const T& value() const & { return std::get<0>(contents_); }
-    T&& value() && { return std::get<0>(std::move(contents_)); }
-    const T&& value() const && { return std::get<0>(std::move(contents_)); }
-
-    const ResultError& error() const & { return std::get<1>(contents_); }
-    ResultError&& error() && { return std::get<1>(std::move(contents_)); }
-    const ResultError&& error() const && { return std::get<1>(std::move(contents_)); }
-
-    const std::string& error_string() const & { return std::get<1>(contents_).error_string; }
-    std::string&& error_string() && { return std::get<1>(std::move(contents_)).error_string; }
-    const std::string&& error_string() const && {
-        return std::get<1>(std::move(contents_)).error_string;
-    }
-
-    int error_errno() const { return std::get<1>(contents_).error_errno; }
-
-    explicit operator bool() const { return has_value(); }
-
-    T& operator*() & { return value(); }
-    const T& operator*() const & { return value(); }
-    T&& operator*() && { return std::move(value()); }
-    const T&& operator*() const && { return std::move(value()); }
-
-    T* operator->() { return &value(); }
-    const T* operator->() const { return &value(); }
-
-  private:
-    std::variant<T, ResultError> contents_;
-};
+using Result = android::base::expected<T, ResultError>;
 
 using Success = std::monostate;
 
 }  // namespace init
 }  // namespace android
 
-#endif
diff --git a/init/result_test.cpp b/init/result_test.cpp
index 327b444..d3d04a0 100644
--- a/init/result_test.cpp
+++ b/init/result_test.cpp
@@ -74,8 +74,8 @@
     ASSERT_FALSE(result);
     ASSERT_FALSE(result.has_value());
 
-    EXPECT_EQ(0, result.error_errno());
-    EXPECT_EQ("failure1", result.error_string());
+    EXPECT_EQ(0, result.error().as_errno);
+    EXPECT_EQ("failure1", result.error().as_string);
 }
 
 TEST(result, result_error_empty) {
@@ -83,8 +83,8 @@
     ASSERT_FALSE(result);
     ASSERT_FALSE(result.has_value());
 
-    EXPECT_EQ(0, result.error_errno());
-    EXPECT_EQ("", result.error_string());
+    EXPECT_EQ(0, result.error().as_errno);
+    EXPECT_EQ("", result.error().as_string);
 }
 
 TEST(result, result_error_rvalue) {
@@ -98,8 +98,8 @@
     ASSERT_FALSE(MakeRvalueErrorResult());
     ASSERT_FALSE(MakeRvalueErrorResult().has_value());
 
-    EXPECT_EQ(0, MakeRvalueErrorResult().error_errno());
-    EXPECT_EQ("failure1", MakeRvalueErrorResult().error_string());
+    EXPECT_EQ(0, MakeRvalueErrorResult().error().as_errno);
+    EXPECT_EQ("failure1", MakeRvalueErrorResult().error().as_string);
 }
 
 TEST(result, result_errno_error) {
@@ -110,8 +110,8 @@
     ASSERT_FALSE(result);
     ASSERT_FALSE(result.has_value());
 
-    EXPECT_EQ(test_errno, result.error_errno());
-    EXPECT_EQ("failure1: "s + strerror(test_errno), result.error_string());
+    EXPECT_EQ(test_errno, result.error().as_errno);
+    EXPECT_EQ("failure1: "s + strerror(test_errno), result.error().as_string);
 }
 
 TEST(result, result_errno_error_no_text) {
@@ -122,8 +122,8 @@
     ASSERT_FALSE(result);
     ASSERT_FALSE(result.has_value());
 
-    EXPECT_EQ(test_errno, result.error_errno());
-    EXPECT_EQ(strerror(test_errno), result.error_string());
+    EXPECT_EQ(test_errno, result.error().as_errno);
+    EXPECT_EQ(strerror(test_errno), result.error().as_string);
 }
 
 TEST(result, result_error_from_other_result) {
@@ -138,8 +138,8 @@
     ASSERT_FALSE(result2);
     ASSERT_FALSE(result2.has_value());
 
-    EXPECT_EQ(0, result.error_errno());
-    EXPECT_EQ(error_text, result.error_string());
+    EXPECT_EQ(0, result.error().as_errno);
+    EXPECT_EQ(error_text, result.error().as_string);
 }
 
 TEST(result, result_error_through_ostream) {
@@ -154,8 +154,8 @@
     ASSERT_FALSE(result2);
     ASSERT_FALSE(result2.has_value());
 
-    EXPECT_EQ(0, result.error_errno());
-    EXPECT_EQ(error_text, result.error_string());
+    EXPECT_EQ(0, result.error().as_errno);
+    EXPECT_EQ(error_text, result.error().as_string);
 }
 
 TEST(result, result_errno_error_through_ostream) {
@@ -174,12 +174,12 @@
     ASSERT_FALSE(result2);
     ASSERT_FALSE(result2.has_value());
 
-    EXPECT_EQ(test_errno, result.error_errno());
-    EXPECT_EQ(error_text + ": " + strerror(test_errno), result.error_string());
+    EXPECT_EQ(test_errno, result.error().as_errno);
+    EXPECT_EQ(error_text + ": " + strerror(test_errno), result.error().as_string);
 }
 
 TEST(result, constructor_forwarding) {
-    auto result = Result<std::string>(5, 'a');
+    auto result = Result<std::string>(std::in_place, 5, 'a');
 
     ASSERT_TRUE(result);
     ASSERT_TRUE(result.has_value());
@@ -298,8 +298,8 @@
     auto result = return_result_result_with_error();
     ASSERT_TRUE(result);
     ASSERT_FALSE(*result);
-    EXPECT_EQ("failure string", result->error_string());
-    EXPECT_EQ(6, result->error_errno());
+    EXPECT_EQ("failure string", (*result).error().as_string);
+    EXPECT_EQ(6, (*result).error().as_errno);
 }
 
 // This test requires that we disable the forwarding reference constructor if Result<T> is the
@@ -312,7 +312,9 @@
         int value_;
     };
 
-    auto return_test_struct = []() -> Result<TestStruct> { return {Result<TestStruct>(6), 6}; };
+    auto return_test_struct = []() -> Result<TestStruct> {
+        return Result<TestStruct>(std::in_place, Result<TestStruct>(std::in_place, 6), 6);
+    };
 
     auto result = return_test_struct();
     ASSERT_TRUE(result);
@@ -326,7 +328,7 @@
 
 TEST(result, die_on_get_error_succesful_result) {
     Result<std::string> result = "success";
-    ASSERT_DEATH(result.error_string(), "");
+    ASSERT_DEATH(result.error(), "");
 }
 
 }  // namespace init
diff --git a/init/rlimit_parser.cpp b/init/rlimit_parser.cpp
index 1e0754a..476a46a 100644
--- a/init/rlimit_parser.cpp
+++ b/init/rlimit_parser.cpp
@@ -77,7 +77,7 @@
         return Error() << "Could not parse hard limit '" << args[3] << "'";
     }
 
-    return {resource, limit};
+    return std::pair{resource, limit};
 }
 
 }  // namespace init
diff --git a/init/rlimit_parser_test.cpp b/init/rlimit_parser_test.cpp
index 659ba8a..e690bf6 100644
--- a/init/rlimit_parser_test.cpp
+++ b/init/rlimit_parser_test.cpp
@@ -43,8 +43,8 @@
     auto result = ParseRlimit(input);
 
     ASSERT_FALSE(result) << "input: " << input[1];
-    EXPECT_EQ(expected_result, result.error_string());
-    EXPECT_EQ(0, result.error_errno());
+    EXPECT_EQ(expected_result, result.error().as_string);
+    EXPECT_EQ(0, result.error().as_errno);
 }
 
 TEST(rlimit, RlimitSuccess) {
diff --git a/init/service.cpp b/init/service.cpp
index ccc37b7..6887d7b 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -1383,7 +1383,7 @@
             continue;
         }
         if (auto result = service->Start(); !result) {
-            LOG(ERROR) << result.error_string();
+            LOG(ERROR) << result.error().as_string;
         }
     }
     delayed_service_names_.clear();
diff --git a/init/subcontext.cpp b/init/subcontext.cpp
index 092c51c..467b0d2 100644
--- a/init/subcontext.cpp
+++ b/init/subcontext.cpp
@@ -141,8 +141,8 @@
         reply->set_success(true);
     } else {
         auto* failure = reply->mutable_failure();
-        failure->set_error_string(result.error_string());
-        failure->set_error_errno(result.error_errno());
+        failure->set_error_string(result.error().as_string);
+        failure->set_error_errno(result.error().as_errno);
     }
 }
 
@@ -177,7 +177,7 @@
 
         auto init_message = ReadMessage(init_fd_);
         if (!init_message) {
-            if (init_message.error_errno() == 0) {
+            if (init_message.error().as_errno == 0) {
                 // If the init file descriptor was closed, let's exit quietly. If
                 // this was accidental, init will restart us. If init died, this
                 // avoids calling abort(3) unnecessarily.
diff --git a/init/subcontext_benchmark.cpp b/init/subcontext_benchmark.cpp
index eae03e3..6307993 100644
--- a/init/subcontext_benchmark.cpp
+++ b/init/subcontext_benchmark.cpp
@@ -39,7 +39,7 @@
     free(context);
 
     while (state.KeepRunning()) {
-        subcontext.Execute(std::vector<std::string>{"return_success"}).IgnoreError();
+        subcontext.Execute(std::vector<std::string>{"return_success"});
     }
 
     if (subcontext.pid() > 0) {
diff --git a/init/subcontext_test.cpp b/init/subcontext_test.cpp
index 230203a..c0662a4 100644
--- a/init/subcontext_test.cpp
+++ b/init/subcontext_test.cpp
@@ -69,7 +69,7 @@
         auto result = subcontext.Execute(std::vector<std::string>{"return_pids_as_error"});
         ASSERT_FALSE(result);
 
-        auto pids = Split(result.error_string(), " ");
+        auto pids = Split(result.error().as_string, " ");
         ASSERT_EQ(2U, pids.size());
         auto our_pid = std::to_string(getpid());
         EXPECT_NE(our_pid, pids[0]);
@@ -116,7 +116,7 @@
 
         auto result = subcontext.Execute(std::vector<std::string>{"return_words_as_error"});
         ASSERT_FALSE(result);
-        EXPECT_EQ(Join(expected_words, " "), result.error_string());
+        EXPECT_EQ(Join(expected_words, " "), result.error().as_string);
         EXPECT_EQ(first_pid, subcontext.pid());
     });
 }
@@ -130,7 +130,7 @@
 
         auto result2 = subcontext.Execute(std::vector<std::string>{"generate_sane_error"});
         ASSERT_FALSE(result2);
-        EXPECT_EQ("Sane error!", result2.error_string());
+        EXPECT_EQ("Sane error!", result2.error().as_string);
         EXPECT_NE(subcontext.pid(), first_pid);
     });
 }
@@ -139,7 +139,7 @@
     RunTest([](auto& subcontext, auto& context_string) {
         auto result = subcontext.Execute(std::vector<std::string>{"return_context_as_error"});
         ASSERT_FALSE(result);
-        ASSERT_EQ(context_string, result.error_string());
+        ASSERT_EQ(context_string, result.error().as_string);
     });
 }
 
@@ -167,7 +167,7 @@
         };
         auto result = subcontext.ExpandArgs(args);
         ASSERT_FALSE(result);
-        EXPECT_EQ("Failed to expand '" + args[1] + "'", result.error_string());
+        EXPECT_EQ("Failed to expand '" + args[1] + "'", result.error().as_string);
     });
 }
 
diff --git a/init/util_test.cpp b/init/util_test.cpp
index 1b5afba..8bf672c 100644
--- a/init/util_test.cpp
+++ b/init/util_test.cpp
@@ -34,7 +34,7 @@
     auto file_contents = ReadFile("/proc/does-not-exist");
     EXPECT_EQ(ENOENT, errno);
     ASSERT_FALSE(file_contents);
-    EXPECT_EQ("open() failed: No such file or directory", file_contents.error_string());
+    EXPECT_EQ("open() failed: No such file or directory", file_contents.error().as_string);
 }
 
 TEST(util, ReadFileGroupWriteable) {
@@ -45,7 +45,7 @@
     EXPECT_NE(-1, fchmodat(AT_FDCWD, tf.path, 0620, AT_SYMLINK_NOFOLLOW)) << strerror(errno);
     auto file_contents = ReadFile(tf.path);
     ASSERT_FALSE(file_contents) << strerror(errno);
-    EXPECT_EQ("Skipping insecure file", file_contents.error_string());
+    EXPECT_EQ("Skipping insecure file", file_contents.error().as_string);
 }
 
 TEST(util, ReadFileWorldWiteable) {
@@ -56,7 +56,7 @@
     EXPECT_NE(-1, fchmodat(AT_FDCWD, tf.path, 0602, AT_SYMLINK_NOFOLLOW)) << strerror(errno);
     auto file_contents = ReadFile(tf.path);
     ASSERT_FALSE(file_contents) << strerror(errno);
-    EXPECT_EQ("Skipping insecure file", file_contents.error_string());
+    EXPECT_EQ("Skipping insecure file", file_contents.error().as_string);
 }
 
 TEST(util, ReadFileSymbolicLink) {
@@ -65,7 +65,8 @@
     auto file_contents = ReadFile("/charger");
     EXPECT_EQ(ELOOP, errno);
     ASSERT_FALSE(file_contents);
-    EXPECT_EQ("open() failed: Too many symbolic links encountered", file_contents.error_string());
+    EXPECT_EQ("open() failed: Too many symbolic links encountered",
+              file_contents.error().as_string);
 }
 
 TEST(util, ReadFileSuccess) {
@@ -130,7 +131,7 @@
 
     decoded_uid = DecodeUid("toot");
     EXPECT_FALSE(decoded_uid);
-    EXPECT_EQ("getpwnam failed: No such file or directory", decoded_uid.error_string());
+    EXPECT_EQ("getpwnam failed: No such file or directory", decoded_uid.error().as_string);
 
     decoded_uid = DecodeUid("123");
     EXPECT_TRUE(decoded_uid);
diff --git a/libcutils/fs_config.cpp b/libcutils/fs_config.cpp
index 494a06f..6606030 100644
--- a/libcutils/fs_config.cpp
+++ b/libcutils/fs_config.cpp
@@ -158,8 +158,9 @@
     { 00750, AID_ROOT,      AID_SHELL,     0, "data/nativetest64/*" },
     { 00600, AID_ROOT,      AID_ROOT,      0, "default.prop" }, // legacy
     { 00600, AID_ROOT,      AID_ROOT,      0, "system/etc/prop.default" },
-    { 00600, AID_ROOT,      AID_ROOT,      0, "odm/build.prop" },
-    { 00600, AID_ROOT,      AID_ROOT,      0, "odm/default.prop" },
+    { 00600, AID_ROOT,      AID_ROOT,      0, "odm/build.prop" }, // legacy; only for P release
+    { 00600, AID_ROOT,      AID_ROOT,      0, "odm/default.prop" }, // legacy; only for P release
+    { 00600, AID_ROOT,      AID_ROOT,      0, "odm/etc/build.prop" },
     { 00444, AID_ROOT,      AID_ROOT,      0, odm_conf_dir + 1 },
     { 00444, AID_ROOT,      AID_ROOT,      0, odm_conf_file + 1 },
     { 00444, AID_ROOT,      AID_ROOT,      0, oem_conf_dir + 1 },
diff --git a/libmemunreachable/tests/MemUnreachable_test.cpp b/libmemunreachable/tests/MemUnreachable_test.cpp
index e464872..9cb1623 100644
--- a/libmemunreachable/tests/MemUnreachable_test.cpp
+++ b/libmemunreachable/tests/MemUnreachable_test.cpp
@@ -47,7 +47,8 @@
 
 // Trick the compiler into thinking a value on the stack is still referenced.
 static void Ref(void** ptr) {
-  write(0, ptr, 0);
+  void** volatile storage;
+  storage = ptr;
 }
 
 class MemunreachableTest : public ::testing::Test {
diff --git a/libziparchive/include/ziparchive/zip_archive.h b/libziparchive/include/ziparchive/zip_archive.h
index 46c982d..a56a4a2 100644
--- a/libziparchive/include/ziparchive/zip_archive.h
+++ b/libziparchive/include/ziparchive/zip_archive.h
@@ -25,6 +25,7 @@
 #include <sys/cdefs.h>
 #include <sys/types.h>
 
+#include <string>
 #include <string_view>
 
 #include "android-base/off64_t.h"
@@ -35,6 +36,7 @@
   kCompressDeflated = 8,  // standard deflate
 };
 
+// TODO: remove this when everyone's moved over to std::string.
 struct ZipString {
   const uint8_t* name;
   uint16_t name_length;
@@ -187,6 +189,8 @@
  * Returns 0 on success, -1 if there are no more elements in this
  * archive and lower negative values on failure.
  */
+int32_t Next(void* cookie, ZipEntry* data, std::string* name);
+// TODO: remove this when everyone's moved over to std::string.
 int32_t Next(void* cookie, ZipEntry* data, ZipString* name);
 
 /*
diff --git a/libziparchive/unzip.cpp b/libziparchive/unzip.cpp
index 3a3a694..426325e 100644
--- a/libziparchive/unzip.cpp
+++ b/libziparchive/unzip.cpp
@@ -255,9 +255,8 @@
   }
 
   ZipEntry entry;
-  ZipString string;
-  while ((err = Next(cookie, &entry, &string)) >= 0) {
-    std::string name(string.name, string.name + string.name_length);
+  std::string name;
+  while ((err = Next(cookie, &entry, &name)) >= 0) {
     if (ShouldInclude(name)) ProcessOne(zah, entry, name);
   }
 
diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index 269e4ca..f4b6c74 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -747,6 +747,15 @@
   return FindEntry(archive, static_cast<uint32_t>(ent), data);
 }
 
+int32_t Next(void* cookie, ZipEntry* data, std::string* name) {
+  ZipString zs;
+  int32_t result = Next(cookie, data, &zs);
+  if (result == 0) {
+    *name = std::string(reinterpret_cast<const char*>(zs.name), zs.name_length);
+  }
+  return result;
+}
+
 int32_t Next(void* cookie, ZipEntry* data, ZipString* name) {
   IterationHandle* handle = reinterpret_cast<IterationHandle*>(cookie);
   if (handle == NULL) {
diff --git a/libziparchive/zip_archive_benchmark.cpp b/libziparchive/zip_archive_benchmark.cpp
index 434f2e1..23ed408 100644
--- a/libziparchive/zip_archive_benchmark.cpp
+++ b/libziparchive/zip_archive_benchmark.cpp
@@ -71,7 +71,7 @@
   ZipArchiveHandle handle;
   void* iteration_cookie;
   ZipEntry data;
-  ZipString name;
+  std::string name;
 
   while (state.KeepRunning()) {
     OpenArchive(temp_file->path, &handle);
diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc
index 993c975..b6ca9ec 100644
--- a/libziparchive/zip_archive_test.cc
+++ b/libziparchive/zip_archive_test.cc
@@ -118,10 +118,10 @@
   ZipEntry data;
   std::vector<std::string> names;
 
-  ZipString name;
+  std::string name;
   for (size_t i = 0; i < expected_names_sorted.size(); ++i) {
     ASSERT_EQ(0, Next(iteration_cookie, &data, &name));
-    names.push_back(std::string(reinterpret_cast<const char*>(name.name), name.name_length));
+    names.push_back(name);
   }
 
   // End of iteration.
@@ -167,7 +167,7 @@
   ASSERT_EQ(0, StartIteration(handle, &iteration_cookie, "x", "y"));
 
   ZipEntry data;
-  ZipString name;
+  std::string name;
 
   // End of iteration.
   ASSERT_EQ(-1, Next(iteration_cookie, &data, &name));
@@ -224,7 +224,7 @@
   void* iteration_cookie;
   ASSERT_EQ(0, StartIteration(handle, &iteration_cookie));
 
-  ZipString name;
+  std::string name;
   ZipEntry data;
 
   ASSERT_EQ(Next(iteration_cookie, &data, &name), 0);