idmap2: replace std::pair<bool, T> with Result<T>
Introduce a new type Result<T> to indicate if an operation succeeded or
not, and if it did, to hold the return value of the operation. This is
the same as how std::pair<bool, T> is already used in the codebase, so
replace all instances with Result<T> to improve clarity.
Result<T> is simply an alias for std::optional<T>. The difference is
semantic: use Result<T> as the return value for functions that can fail,
use std::optional<T> when values are truly optional. This is modelled
after Rust's std::result and std::option.
A future change may graduate Result<T> to a proper class which can hold
additional details on why an operation failed, such as a string or an
error code. As a special case, continue to use std::unique_ptr<T>
instead of Result<std::unique_ptr<T>> for now: the latter would increase
code complexity without added benefit.
Test: make idmap2_tests
Change-Id: I2a8355107ed2b6485409e5e655a84cf1e20b9911
diff --git a/cmds/idmap2/libidmap2/Idmap.cpp b/cmds/idmap2/libidmap2/Idmap.cpp
index 5a47e30..1ef3267 100644
--- a/cmds/idmap2/libidmap2/Idmap.cpp
+++ b/cmds/idmap2/libidmap2/Idmap.cpp
@@ -33,6 +33,7 @@
#include "idmap2/Idmap.h"
#include "idmap2/ResourceUtils.h"
+#include "idmap2/Result.h"
#include "idmap2/ZipFile.h"
namespace android {
@@ -143,18 +144,16 @@
return false;
}
- bool status;
- uint32_t target_crc;
- std::tie(status, target_crc) = target_zip->Crc("resources.arsc");
- if (!status) {
+ Result<uint32_t> target_crc = target_zip->Crc("resources.arsc");
+ if (!target_crc) {
out_error << "error: failed to get target crc" << std::endl;
return false;
}
- if (target_crc_ != target_crc) {
+ if (target_crc_ != *target_crc) {
out_error << base::StringPrintf(
"error: bad target crc: idmap version 0x%08x, file system version 0x%08x",
- target_crc_, target_crc)
+ target_crc_, *target_crc)
<< std::endl;
return false;
}
@@ -165,17 +164,16 @@
return false;
}
- uint32_t overlay_crc;
- std::tie(status, overlay_crc) = overlay_zip->Crc("resources.arsc");
- if (!status) {
+ Result<uint32_t> overlay_crc = overlay_zip->Crc("resources.arsc");
+ if (!overlay_crc) {
out_error << "error: failed to get overlay crc" << std::endl;
return false;
}
- if (overlay_crc_ != overlay_crc) {
+ if (overlay_crc_ != *overlay_crc) {
out_error << base::StringPrintf(
"error: bad overlay crc: idmap version 0x%08x, file system version 0x%08x",
- overlay_crc_, overlay_crc)
+ overlay_crc_, *overlay_crc)
<< std::endl;
return false;
}
@@ -322,17 +320,20 @@
std::unique_ptr<IdmapHeader> header(new IdmapHeader());
header->magic_ = kIdmapMagic;
header->version_ = kIdmapCurrentVersion;
- bool crc_status;
- std::tie(crc_status, header->target_crc_) = target_zip->Crc("resources.arsc");
- if (!crc_status) {
+
+ Result<uint32_t> crc = target_zip->Crc("resources.arsc");
+ if (!crc) {
out_error << "error: failed to get zip crc for target" << std::endl;
return nullptr;
}
- std::tie(crc_status, header->overlay_crc_) = overlay_zip->Crc("resources.arsc");
- if (!crc_status) {
+ header->target_crc_ = *crc;
+
+ crc = overlay_zip->Crc("resources.arsc");
+ if (!crc) {
out_error << "error: failed to get zip crc for overlay" << std::endl;
return nullptr;
}
+ header->overlay_crc_ = *crc;
if (target_apk_path.size() > sizeof(header->target_path_)) {
out_error << "error: target apk path \"" << target_apk_path << "\" longer that maximum size "
@@ -358,15 +359,14 @@
const auto end = overlay_pkg->end();
for (auto iter = overlay_pkg->begin(); iter != end; ++iter) {
const ResourceId overlay_resid = *iter;
- bool lookup_ok;
- std::string name;
- std::tie(lookup_ok, name) = utils::ResToTypeEntryName(overlay_asset_manager, overlay_resid);
- if (!lookup_ok) {
+ Result<std::string> name = utils::ResToTypeEntryName(overlay_asset_manager, overlay_resid);
+ if (!name) {
continue;
}
// prepend "<package>:" to turn name into "<package>:<type>/<name>"
- name = base::StringPrintf("%s:%s", target_pkg->GetPackageName().c_str(), name.c_str());
- const ResourceId target_resid = NameToResid(target_asset_manager, name);
+ const std::string full_name =
+ base::StringPrintf("%s:%s", target_pkg->GetPackageName().c_str(), name->c_str());
+ const ResourceId target_resid = NameToResid(target_asset_manager, full_name);
if (target_resid == 0) {
continue;
}
diff --git a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
index 492e6f0..fb3bc5b 100644
--- a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
+++ b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
@@ -15,7 +15,6 @@
*/
#include <string>
-#include <utility>
#include "android-base/macros.h"
#include "android-base/stringprintf.h"
@@ -23,6 +22,7 @@
#include "idmap2/PrettyPrintVisitor.h"
#include "idmap2/ResourceUtils.h"
+#include "idmap2/Result.h"
namespace android {
namespace idmap2 {
@@ -63,11 +63,9 @@
stream_ << base::StringPrintf("0x%08x -> 0x%08x", target_resid, overlay_resid);
if (target_package_loaded) {
- bool lookup_ok;
- std::string name;
- std::tie(lookup_ok, name) = utils::ResToTypeEntryName(target_am_, target_resid);
- if (lookup_ok) {
- stream_ << " " << name;
+ Result<std::string> name = utils::ResToTypeEntryName(target_am_, target_resid);
+ if (name) {
+ stream_ << " " << *name;
}
}
stream_ << std::endl;
diff --git a/cmds/idmap2/libidmap2/RawPrintVisitor.cpp b/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
index 57cfc8e..7c24445 100644
--- a/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
+++ b/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
@@ -16,7 +16,6 @@
#include <cstdarg>
#include <string>
-#include <utility>
#include "android-base/macros.h"
#include "android-base/stringprintf.h"
@@ -24,6 +23,7 @@
#include "idmap2/RawPrintVisitor.h"
#include "idmap2/ResourceUtils.h"
+#include "idmap2/Result.h"
using android::ApkAssets;
@@ -75,14 +75,13 @@
const ResourceId target_resid =
RESID(last_seen_package_id_, te.GetTargetTypeId(), te.GetEntryOffset() + i);
const ResourceId overlay_resid = RESID(last_seen_package_id_, te.GetOverlayTypeId(), entry);
- bool lookup_ok = false;
- std::string name;
+ Result<std::string> name;
if (target_package_loaded) {
- std::tie(lookup_ok, name) = utils::ResToTypeEntryName(target_am_, target_resid);
+ name = utils::ResToTypeEntryName(target_am_, target_resid);
}
- if (lookup_ok) {
+ if (name) {
print(static_cast<uint32_t>(entry), "0x%08x -> 0x%08x %s", target_resid, overlay_resid,
- name.c_str());
+ name->c_str());
} else {
print(static_cast<uint32_t>(entry), "0x%08x -> 0x%08x", target_resid, overlay_resid);
}
diff --git a/cmds/idmap2/libidmap2/ResourceUtils.cpp b/cmds/idmap2/libidmap2/ResourceUtils.cpp
index e98f843..5c89783 100644
--- a/cmds/idmap2/libidmap2/ResourceUtils.cpp
+++ b/cmds/idmap2/libidmap2/ResourceUtils.cpp
@@ -15,12 +15,12 @@
*/
#include <string>
-#include <utility>
#include "androidfw/StringPiece.h"
#include "androidfw/Util.h"
#include "idmap2/ResourceUtils.h"
+#include "idmap2/Result.h"
using android::StringPiece16;
using android::util::Utf16ToUtf8;
@@ -29,11 +29,10 @@
namespace idmap2 {
namespace utils {
-std::pair<bool, std::string> WARN_UNUSED ResToTypeEntryName(const AssetManager2& am,
- ResourceId resid) {
+Result<std::string> WARN_UNUSED ResToTypeEntryName(const AssetManager2& am, ResourceId resid) {
AssetManager2::ResourceName name;
if (!am.GetResourceName(resid, &name)) {
- return std::make_pair(false, "");
+ return {};
}
std::string out;
if (name.type != nullptr) {
@@ -47,7 +46,7 @@
} else {
out += Utf16ToUtf8(StringPiece16(name.entry16, name.entry_len));
}
- return std::make_pair(true, out);
+ return {out};
}
} // namespace utils
diff --git a/cmds/idmap2/libidmap2/ZipFile.cpp b/cmds/idmap2/libidmap2/ZipFile.cpp
index 3f2079a..9fb611d 100644
--- a/cmds/idmap2/libidmap2/ZipFile.cpp
+++ b/cmds/idmap2/libidmap2/ZipFile.cpp
@@ -16,8 +16,8 @@
#include <memory>
#include <string>
-#include <utility>
+#include "idmap2/Result.h"
#include "idmap2/ZipFile.h"
namespace android {
@@ -57,10 +57,10 @@
return chunk;
}
-std::pair<bool, uint32_t> ZipFile::Crc(const std::string& entryPath) const {
+Result<uint32_t> ZipFile::Crc(const std::string& entryPath) const {
::ZipEntry entry;
int32_t status = ::FindEntry(handle_, ::ZipString(entryPath.c_str()), &entry);
- return std::make_pair(status == 0, entry.crc32);
+ return status == 0 ? Result<uint32_t>(entry.crc32) : kResultError;
}
} // namespace idmap2