Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

idmap2: replace std::pair<bool, T> with Result<T>

Browse Source 
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
This commit is contained in:
Mårten Kongstad
2018-11-19 14:14:37 +01:00
committed by Todd Kennedy
parent b57794af28
commit 0f76311c1d
11 changed files with 112 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
cmds/idmap2/idmap2/Lookup.cpp
View File

@@ -36,6 +36,7 @@
#include "idmap2/CommandLineOptions.h"
#include "idmap2/Idmap.h"
#include "idmap2/Result.h"
#include "idmap2/Xml.h"
#include "idmap2/ZipFile.h"
@@ -53,39 +54,40 @@ using android::base::StringPrintf;
using android::idmap2::CommandLineOptions;
using android::idmap2::IdmapHeader;
using android::idmap2::ResourceId;
using android::idmap2::Result;
using android::idmap2::Xml;
using android::idmap2::ZipFile;
using android::util::Utf16ToUtf8;
namespace {
std::pair<bool, ResourceId> WARN_UNUSED ParseResReference(const AssetManager2& am,
const std::string& res,
const std::string& fallback_package) {
Result<ResourceId> WARN_UNUSED ParseResReference(const AssetManager2& am, const std::string& res,
const std::string& fallback_package) {
// first, try to parse as a hex number
char* endptr = nullptr;
ResourceId resid;
resid = strtol(res.c_str(), &endptr, 16);
if (*endptr == '\0') {
return std::make_pair(true, resid);
return {resid};
}
// next, try to parse as a package:type/name string
resid = am.GetResourceId(res, "", fallback_package);
if (is_valid_resid(resid)) {
return std::make_pair(true, resid);
return {resid};
}
// end of the road: res could not be parsed
return std::make_pair(false, 0);
return {};
}
std::pair<bool, std::string> WARN_UNUSED GetValue(const AssetManager2& am, ResourceId resid) {
Result<std::string> WARN_UNUSED GetValue(const AssetManager2& am, ResourceId resid) {
Res_value value;
ResTable_config config;
uint32_t flags;
ApkAssetsCookie cookie = am.GetResource(resid, false, 0, &value, &config, &flags);
if (cookie == kInvalidCookie) {
return std::make_pair(false, "");
return {};
}
std::string out;
@@ -123,31 +125,31 @@ std::pair<bool, std::string> WARN_UNUSED GetValue(const AssetManager2& am, Resou
out.append(StringPrintf("dataType=0x%02x data=0x%08x", value.dataType, value.data));
break;
}
return std::make_pair(true, out);
return {out};
}
std::pair<bool, std::string> GetTargetPackageNameFromManifest(const std::string& apk_path) {
Result<std::string> GetTargetPackageNameFromManifest(const std::string& apk_path) {
const auto zip = ZipFile::Open(apk_path);
if (!zip) {
return std::make_pair(false, "");
return {};
}
const auto entry = zip->Uncompress("AndroidManifest.xml");
if (!entry) {
return std::make_pair(false, "");
return {};
}
const auto xml = Xml::Create(entry->buf, entry->size);
if (!xml) {
return std::make_pair(false, "");
return {};
}
const auto tag = xml->FindTag("overlay");
if (!tag) {
return std::make_pair(false, "");
return {};
}
const auto iter = tag->find("targetPackage");
if (iter == tag->end()) {
return std::make_pair(false, "");
return {};
}
return std::make_pair(true, iter->second);
return {iter->second};
}
} // namespace
@@ -195,14 +197,14 @@ bool Lookup(const std::vector<std::string>& args, std::ostream& out_error) {
}
apk_assets.push_back(std::move(target_apk));
bool lookup_ok;
std::tie(lookup_ok, target_package_name) =
const Result<std::string> package_name =
GetTargetPackageNameFromManifest(idmap_header->GetOverlayPath().to_string());
if (!lookup_ok) {
if (!package_name) {
out_error << "error: failed to parse android:targetPackage from overlay manifest"
<< std::endl;
return false;
}
target_package_name = *package_name;
} else if (target_path != idmap_header->GetTargetPath()) {
out_error << "error: different target APKs (expected target APK " << target_path << " but "
<< idmap_path << " has target APK " << idmap_header->GetTargetPath() << ")"
@@ -227,21 +229,18 @@ bool Lookup(const std::vector<std::string>& args, std::ostream& out_error) {
am.SetApkAssets(raw_pointer_apk_assets);
am.SetConfiguration(config);
ResourceId resid;
bool lookup_ok;
std::tie(lookup_ok, resid) = ParseResReference(am, resid_str, target_package_name);
if (!lookup_ok) {
const Result<ResourceId> resid = ParseResReference(am, resid_str, target_package_name);
if (!resid) {
out_error << "error: failed to parse resource ID" << std::endl;
return false;
}
std::string value;
std::tie(lookup_ok, value) = GetValue(am, resid);
if (!lookup_ok) {
out_error << StringPrintf("error: resource 0x%08x not found", resid) << std::endl;
const Result<std::string> value = GetValue(am, *resid);
if (!value) {
out_error << StringPrintf("error: resource 0x%08x not found", *resid) << std::endl;
return false;
}
std::cout << value << std::endl;
std::cout << *value << std::endl;
return true;
}

5
cmds/idmap2/include/idmap2/ResourceUtils.h
View File

@@ -18,19 +18,18 @@
#define IDMAP2_INCLUDE_IDMAP2_RESOURCEUTILS_H_
#include <string>
#include <utility>
#include "android-base/macros.h"
#include "androidfw/AssetManager2.h"
#include "idmap2/Idmap.h"
#include "idmap2/Result.h"
namespace android {
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);
} // namespace utils
} // namespace idmap2

31
cmds/idmap2/include/idmap2/Result.h Normal file
View File

@@ -0,0 +1,31 @@
/*
* Copyright (C) 2018 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 IDMAP2_INCLUDE_IDMAP2_RESULT_H_
#define IDMAP2_INCLUDE_IDMAP2_RESULT_H_
#include <optional>
namespace android::idmap2 {
template <typename T>
using Result = std::optional<T>;
static constexpr std::nullopt_t kResultError = std::nullopt;
} // namespace android::idmap2
#endif // IDMAP2_INCLUDE_IDMAP2_RESULT_H_

4
cmds/idmap2/include/idmap2/ZipFile.h
View File

@@ -19,10 +19,10 @@
#include <memory>
#include <string>
#include <utility>
#include "android-base/macros.h"
#include "ziparchive/zip_archive.h"
#include "idmap2/Result.h"
namespace android {
namespace idmap2 {
@@ -43,7 +43,7 @@ class ZipFile {
static std::unique_ptr<const ZipFile> Open(const std::string& path);
std::unique_ptr<const MemoryChunk> Uncompress(const std::string& entryPath) const;
std::pair<bool, uint32_t> Crc(const std::string& entryPath) const;
Result<uint32_t> Crc(const std::string& entryPath) const;
~ZipFile();

44
cmds/idmap2/libidmap2/Idmap.cpp
View File

@@ -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 @@ bool IdmapHeader::IsUpToDate(std::ostream& out_error) const {
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 @@ bool IdmapHeader::IsUpToDate(std::ostream& out_error) const {
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<const Idmap> Idmap::FromApkAssets(const std::string& target_apk_
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 @@ std::unique_ptr<const Idmap> Idmap::FromApkAssets(const std::string& target_apk_
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;
}

10
cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
View File

@@ -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 @@ void PrettyPrintVisitor::visit(const IdmapData::TypeEntry& te) {
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;

11
cmds/idmap2/libidmap2/RawPrintVisitor.cpp
View File

@@ -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 @@ void RawPrintVisitor::visit(const IdmapData::TypeEntry& te) {
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);
}

9
cmds/idmap2/libidmap2/ResourceUtils.cpp
View File

@@ -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 android {
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 @@ std::pair<bool, std::string> WARN_UNUSED ResToTypeEntryName(const AssetManager2&
} else {
out += Utf16ToUtf8(StringPiece16(name.entry16, name.entry_len));
}
return std::make_pair(true, out);
return {out};
}
} // namespace utils

6
cmds/idmap2/libidmap2/ZipFile.cpp
View File

@@ -16,8 +16,8 @@
#include <memory>
#include <string>
#include <utility>
#include "idmap2/Result.h"
#include "idmap2/ZipFile.h"
namespace android {
@@ -57,10 +57,10 @@ std::unique_ptr<const MemoryChunk> ZipFile::Uncompress(const std::string& entryP
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

15
cmds/idmap2/tests/ResourceUtilsTests.cpp
View File

@@ -16,13 +16,13 @@
#include <memory>
#include <string>
#include <utility>
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "androidfw/ApkAssets.h"
#include "idmap2/ResourceUtils.h"
#include "idmap2/Result.h"
#include "TestHelpers.h"
@@ -52,17 +52,14 @@ class ResourceUtilsTests : public Idmap2Tests {
};
TEST_F(ResourceUtilsTests, ResToTypeEntryName) {
bool lookup_ok;
std::string name;
std::tie(lookup_ok, name) = utils::ResToTypeEntryName(GetAssetManager(), 0x7f010000u);
ASSERT_TRUE(lookup_ok);
ASSERT_EQ(name, "integer/int1");
Result<std::string> name = utils::ResToTypeEntryName(GetAssetManager(), 0x7f010000u);
ASSERT_TRUE(name);
ASSERT_EQ(*name, "integer/int1");
}
TEST_F(ResourceUtilsTests, ResToTypeEntryNameNoSuchResourceId) {
bool lookup_ok;
std::tie(lookup_ok, std::ignore) = utils::ResToTypeEntryName(GetAssetManager(), 0x7f123456u);
ASSERT_FALSE(lookup_ok);
Result<std::string> name = utils::ResToTypeEntryName(GetAssetManager(), 0x7f123456u);
ASSERT_FALSE(name);
}
} // namespace idmap2

14
cmds/idmap2/tests/ZipFileTests.cpp
View File

@@ -16,8 +16,8 @@
#include <cstdio> // fclose
#include <string>
#include <utility>
#include "idmap2/Result.h"
#include "idmap2/ZipFile.h"
#include "gmock/gmock.h"
@@ -44,14 +44,12 @@ TEST(ZipFileTests, Crc) {
auto zip = ZipFile::Open(GetTestDataPath() + "/target/target.apk");
ASSERT_THAT(zip, NotNull());
bool status;
uint32_t crc;
std::tie(status, crc) = zip->Crc("AndroidManifest.xml");
ASSERT_TRUE(status);
ASSERT_EQ(crc, 0x762f3d24);
Result<uint32_t> crc = zip->Crc("AndroidManifest.xml");
ASSERT_TRUE(crc);
ASSERT_EQ(*crc, 0x762f3d24);
std::tie(status, std::ignore) = zip->Crc("does-not-exist");
ASSERT_FALSE(status);
Result<uint32_t> crc2 = zip->Crc("does-not-exist");
ASSERT_FALSE(crc2);
}
TEST(ZipFileTests, Uncompress) {