3 files changed, 104 insertions, 44 deletions

diff --git a/core/jni/com_android_internal_content_NativeLibraryHelper.cpp b/core/jni/com_android_internal_content_NativeLibraryHelper.cpp
index 9017d58913b3..da308b26fe59 100644
--- a/core/jni/com_android_internal_content_NativeLibraryHelper.cpp
+++ b/core/jni/com_android_internal_content_NativeLibraryHelper.cpp
@@ -292,21 +292,31 @@ private:
     }
 
 public:
-    static NativeLibrariesIterator* create(ZipFileRO* zipFile, bool debuggable) {
-        void* cookie = nullptr;
+    static base::expected<std::unique_ptr<NativeLibrariesIterator>, int32_t> create(
+            ZipFileRO* zipFile, bool debuggable) {
         // Do not specify a suffix to find both .so files and gdbserver.
-        if (!zipFile->startIteration(&cookie, APK_LIB.data(), nullptr /* suffix */)) {
-            return nullptr;
+        auto result = zipFile->startIterationOrError(APK_LIB.data(), nullptr /* suffix */);
+        if (!result.ok()) {
+            return base::unexpected(result.error());
         }
 
-        return new NativeLibrariesIterator(zipFile, debuggable, cookie);
+        return std::unique_ptr<NativeLibrariesIterator>(
+                new NativeLibrariesIterator(zipFile, debuggable, result.value()));
     }
 
-    ZipEntryRO next() {
-        ZipEntryRO next = nullptr;
-        while ((next = mZipFile->nextEntry(mCookie)) != nullptr) {
+    base::expected<ZipEntryRO, int32_t> next() {
+        ZipEntryRO nextEntry;
+        while (true) {
+            auto next = mZipFile->nextEntryOrError(mCookie);
+            if (!next.ok()) {
+                return base::unexpected(next.error());
+            }
+            nextEntry = next.value();
+            if (nextEntry == nullptr) {
+                break;
+            }
             // Make sure this entry has a filename.
-            if (mZipFile->getEntryFileName(next, fileName, sizeof(fileName))) {
+            if (mZipFile->getEntryFileName(nextEntry, fileName, sizeof(fileName))) {
                 continue;
             }
 
@@ -317,7 +327,7 @@ public:
             }
         }
 
-        return next;
+        return nextEntry;
     }
 
     inline const char* currentEntry() const {
@@ -348,19 +358,28 @@ iterateOverNativeFiles(JNIEnv *env, jlong apkHandle, jstring javaCpuAbi,
         return INSTALL_FAILED_INVALID_APK;
     }
 
-    std::unique_ptr<NativeLibrariesIterator> it(
-            NativeLibrariesIterator::create(zipFile, debuggable));
-    if (it.get() == nullptr) {
+    auto result = NativeLibrariesIterator::create(zipFile, debuggable);
+    if (!result.ok()) {
         return INSTALL_FAILED_INVALID_APK;
     }
+    std::unique_ptr<NativeLibrariesIterator> it(std::move(result.value()));
 
     const ScopedUtfChars cpuAbi(env, javaCpuAbi);
     if (cpuAbi.c_str() == nullptr) {
         // This would've thrown, so this return code isn't observable by Java.
         return INSTALL_FAILED_INVALID_APK;
     }
-    ZipEntryRO entry = nullptr;
-    while ((entry = it->next()) != nullptr) {
+
+    while (true) {
+        auto next = it->next();
+        if (!next.ok()) {
+            return INSTALL_FAILED_INVALID_APK;
+        }
+        auto entry = next.value();
+        if (entry == nullptr) {
+            break;
+        }
+
         const char* fileName = it->currentEntry();
         const char* lastSlash = it->lastSlash();
 
@@ -388,11 +407,11 @@ static int findSupportedAbi(JNIEnv* env, jlong apkHandle, jobjectArray supported
         return INSTALL_FAILED_INVALID_APK;
     }
 
-    std::unique_ptr<NativeLibrariesIterator> it(
-            NativeLibrariesIterator::create(zipFile, debuggable));
-    if (it.get() == nullptr) {
+    auto result = NativeLibrariesIterator::create(zipFile, debuggable);
+    if (!result.ok()) {
         return INSTALL_FAILED_INVALID_APK;
     }
+    std::unique_ptr<NativeLibrariesIterator> it(std::move(result.value()));
 
     const int numAbis = env->GetArrayLength(supportedAbisArray);
 
@@ -402,9 +421,17 @@ static int findSupportedAbi(JNIEnv* env, jlong apkHandle, jobjectArray supported
         supportedAbis.emplace_back(env, (jstring)env->GetObjectArrayElement(supportedAbisArray, i));
     }
 
-    ZipEntryRO entry = nullptr;
     int status = NO_NATIVE_LIBRARIES;
-    while ((entry = it->next()) != nullptr) {
+    while (true) {
+        auto next = it->next();
+        if (!next.ok()) {
+            return INSTALL_FAILED_INVALID_APK;
+        }
+        auto entry = next.value();
+        if (entry == nullptr) {
+            break;
+        }
+
         // We're currently in the lib/ directory of the APK, so it does have some native
         // code. We should return INSTALL_FAILED_NO_MATCHING_ABIS if none of the
         // libraries match.
diff --git a/libs/androidfw/ZipFileRO.cpp b/libs/androidfw/ZipFileRO.cpp
index 52e7a70521a1..d7b5914130ee 100644
--- a/libs/androidfw/ZipFileRO.cpp
+++ b/libs/androidfw/ZipFileRO.cpp
@@ -40,17 +40,24 @@ class _ZipEntryRO {
 public:
     ZipEntry entry;
     std::string_view name;
-    void *cookie;
+    void *cookie = nullptr;
 
-    _ZipEntryRO() : cookie(NULL) {}
+    _ZipEntryRO() = default;
 
     ~_ZipEntryRO() {
-      EndIteration(cookie);
+        EndIteration(cookie);
+    }
+
+    android::ZipEntryRO convertToPtr() {
+        _ZipEntryRO* result = new _ZipEntryRO;
+        result->entry = std::move(this->entry);
+        result->name = std::move(this->name);
+        result->cookie = std::exchange(this->cookie, nullptr);
+        return result;
     }
 
 private:
-    _ZipEntryRO(const _ZipEntryRO& other);
-    _ZipEntryRO& operator=(const _ZipEntryRO& other);
+    DISALLOW_COPY_AND_ASSIGN(_ZipEntryRO);
 };
 
 ZipFileRO::~ZipFileRO() {
@@ -94,17 +101,15 @@ ZipFileRO::~ZipFileRO() {
 
 ZipEntryRO ZipFileRO::findEntryByName(const char* entryName) const
 {
-    _ZipEntryRO* data = new _ZipEntryRO;
-
-    data->name = entryName;
+    _ZipEntryRO data;
+    data.name = entryName;
 
-    const int32_t error = FindEntry(mHandle, entryName, &(data->entry));
+    const int32_t error = FindEntry(mHandle, entryName, &(data.entry));
     if (error) {
-        delete data;
-        return NULL;
+        return nullptr;
     }
 
-    return (ZipEntryRO) data;
+    return data.convertToPtr();
 }
 
 /*
@@ -143,35 +148,50 @@ bool ZipFileRO::getEntryInfo(ZipEntryRO entry, uint16_t* pMethod,
 }
 
 bool ZipFileRO::startIteration(void** cookie) {
-  return startIteration(cookie, NULL, NULL);
+  return startIteration(cookie, nullptr, nullptr);
 }
 
-bool ZipFileRO::startIteration(void** cookie, const char* prefix, const char* suffix)
-{
-    _ZipEntryRO* ze = new _ZipEntryRO;
-    int32_t error = StartIteration(mHandle, &(ze->cookie),
+bool ZipFileRO::startIteration(void** cookie, const char* prefix, const char* suffix) {
+    auto result = startIterationOrError(prefix, suffix);
+    if (!result.ok()) {
+        return false;
+    }
+    *cookie = result.value();
+    return true;
+}
+
+base::expected<void*, int32_t>
+ZipFileRO::startIterationOrError(const char* prefix, const char* suffix) {
+    _ZipEntryRO ze;
+    int32_t error = StartIteration(mHandle, &(ze.cookie),
                                    prefix ? prefix : "", suffix ? suffix : "");
     if (error) {
         ALOGW("Could not start iteration over %s: %s", mFileName != NULL ? mFileName : "<null>",
                 ErrorCodeString(error));
-        delete ze;
-        return false;
+        return base::unexpected(error);
     }
 
-    *cookie = ze;
-    return true;
+    return ze.convertToPtr();
 }
 
-ZipEntryRO ZipFileRO::nextEntry(void* cookie)
-{
+ZipEntryRO ZipFileRO::nextEntry(void* cookie) {
+    auto result = nextEntryOrError(cookie);
+    if (!result.ok()) {
+        return nullptr;
+    }
+    return result.value();
+}
+
+base::expected<ZipEntryRO, int32_t> ZipFileRO::nextEntryOrError(void* cookie) {
     _ZipEntryRO* ze = reinterpret_cast<_ZipEntryRO*>(cookie);
     int32_t error = Next(ze->cookie, &(ze->entry), &(ze->name));
     if (error) {
         if (error != -1) {
             ALOGW("Error iteration over %s: %s", mFileName != NULL ? mFileName : "<null>",
                     ErrorCodeString(error));
+            return base::unexpected(error);
         }
-        return NULL;
+        return nullptr;
     }
 
     return &(ze->entry);
diff --git a/libs/androidfw/include/androidfw/ZipFileRO.h b/libs/androidfw/include/androidfw/ZipFileRO.h
index 10f6d0655bf4..be1f98f4843d 100644
--- a/libs/androidfw/include/androidfw/ZipFileRO.h
+++ b/libs/androidfw/include/androidfw/ZipFileRO.h
@@ -37,6 +37,8 @@
 #include <unistd.h>
 #include <time.h>
 
+#include <android-base/expected.h>
+
 #include <util/map_ptr.h>
 
 #include <utils/Compat.h>
@@ -102,6 +104,11 @@ public:
      */
     bool startIteration(void** cookie);
     bool startIteration(void** cookie, const char* prefix, const char* suffix);
+    /*
+     * Same as above, but returns the error code in case of failure.
+     * #see libziparchive/zip_error.h.
+     */
+    base::expected<void*, int32_t> startIterationOrError(const char* prefix, const char* suffix);
 
     /**
      * Return the next entry in iteration order, or NULL if there are no more
@@ -109,6 +116,12 @@ public:
      */
     ZipEntryRO nextEntry(void* cookie);
 
+    /**
+     * Same as above, but returns the error code in case of failure.
+     * #see libziparchive/zip_error.h.
+     */
+    base::expected<ZipEntryRO, int32_t> nextEntryOrError(void* cookie);
+
     void endIteration(void* cookie);
 
     void releaseEntry(ZipEntryRO entry) const;