idmap2 code cleanup
- proper use of random number generators in FileUtils
- get rid of <iostream> include where possible
- replace std::endl with '\n' as we don't really need stream
flushes
- added a few missed moves
Bug: 282215580
Test: build, UTs and boot
Change-Id: I2dbe5e7c240acd0f344158ae6d9f2ee2b9c2c6ab
diff --git a/cmds/idmap2/idmap2/CreateMultiple.cpp b/cmds/idmap2/idmap2/CreateMultiple.cpp
index 953d99f..68800cd 100644
--- a/cmds/idmap2/idmap2/CreateMultiple.cpp
+++ b/cmds/idmap2/idmap2/CreateMultiple.cpp
@@ -18,8 +18,8 @@
#include <sys/types.h> // umask
#include <fstream>
+#include <iostream>
#include <memory>
-#include <ostream>
#include <string>
#include <vector>
@@ -51,7 +51,7 @@
Result<Unit> CreateMultiple(const std::vector<std::string>& args) {
SYSTRACE << "CreateMultiple " << args;
std::string target_apk_path;
- std::string idmap_dir = kIdmapCacheDir;
+ std::string idmap_dir{kIdmapCacheDir};
std::vector<std::string> overlay_apk_paths;
std::vector<std::string> policies;
bool ignore_overlayable = false;
@@ -67,7 +67,7 @@
.OptionalOption("--idmap-dir",
StringPrintf("output: path to the directory in which to write idmap file"
" (defaults to %s)",
- kIdmapCacheDir),
+ kIdmapCacheDir.data()),
&idmap_dir)
.OptionalOption("--policy",
"input: an overlayable policy this overlay fulfills"
@@ -142,7 +142,7 @@
}
for (const std::string& idmap_path : idmap_paths) {
- std::cout << idmap_path << std::endl;
+ std::cout << idmap_path << '\n';
}
return Unit{};
diff --git a/cmds/idmap2/idmap2/Lookup.cpp b/cmds/idmap2/idmap2/Lookup.cpp
index add0d8d..3704b4a 100644
--- a/cmds/idmap2/idmap2/Lookup.cpp
+++ b/cmds/idmap2/idmap2/Lookup.cpp
@@ -16,9 +16,9 @@
#include <algorithm>
#include <fstream>
+#include <iostream>
#include <iterator>
#include <memory>
-#include <ostream>
#include <string>
#include <utility>
#include <vector>
@@ -230,7 +230,7 @@
if (!value) {
return Error(value.GetError(), "resource 0x%08x not found", *resid);
}
- std::cout << *value << std::endl;
+ std::cout << *value << '\n';
}
return Unit{};
diff --git a/cmds/idmap2/idmap2/Main.cpp b/cmds/idmap2/idmap2/Main.cpp
index aa6d0e7..5ef15a6 100644
--- a/cmds/idmap2/idmap2/Main.cpp
+++ b/cmds/idmap2/idmap2/Main.cpp
@@ -45,7 +45,7 @@
}
out << iter->first;
}
- out << "]" << std::endl;
+ out << "]" << '\n';
}
} // namespace
@@ -65,18 +65,18 @@
const std::unique_ptr<std::vector<std::string>> args =
CommandLineOptions::ConvertArgvToVector(argc - 1, const_cast<const char**>(argv + 1));
if (!args) {
- std::cerr << "error: failed to parse command line options" << std::endl;
+ std::cerr << "error: failed to parse command line options" << '\n';
return EXIT_FAILURE;
}
const auto iter = commands.find(argv[1]);
if (iter == commands.end()) {
- std::cerr << argv[1] << ": command not found" << std::endl;
+ std::cerr << argv[1] << ": command not found" << '\n';
PrintUsage(commands, std::cerr);
return EXIT_FAILURE;
}
const auto result = iter->second(*args);
if (!result) {
- std::cerr << "error: " << result.GetErrorMessage() << std::endl;
+ std::cerr << "error: " << result.GetErrorMessage() << '\n';
return EXIT_FAILURE;
}
return EXIT_SUCCESS;
diff --git a/cmds/idmap2/idmap2d/Idmap2Service.cpp b/cmds/idmap2/idmap2d/Idmap2Service.cpp
index 10947dc..3b7ab9c 100644
--- a/cmds/idmap2/idmap2d/Idmap2Service.cpp
+++ b/cmds/idmap2/idmap2d/Idmap2Service.cpp
@@ -257,7 +257,7 @@
const std::string random_suffix = RandomStringForPath(kSuffixLength);
file_name = StringPrintf("%s-%s-%s.frro", overlay.packageName.c_str(),
overlay.overlayName.c_str(), random_suffix.c_str());
- path = StringPrintf("%s/%s", kIdmapCacheDir, file_name.c_str());
+ path = StringPrintf("%s/%s", kIdmapCacheDir.data(), file_name.c_str());
// Invoking std::filesystem::exists with a file name greater than 255 characters will cause this
// process to abort since the name exceeds the maximum file name size.
diff --git a/cmds/idmap2/include/idmap2/BinaryStreamVisitor.h b/cmds/idmap2/include/idmap2/BinaryStreamVisitor.h
index 7b38bd1..57af1b6 100644
--- a/cmds/idmap2/include/idmap2/BinaryStreamVisitor.h
+++ b/cmds/idmap2/include/idmap2/BinaryStreamVisitor.h
@@ -18,7 +18,7 @@
#define IDMAP2_INCLUDE_IDMAP2_BINARYSTREAMVISITOR_H_
#include <cstdint>
-#include <iostream>
+#include <ostream>
#include <string>
#include "idmap2/Idmap.h"
diff --git a/cmds/idmap2/include/idmap2/FabricatedOverlay.h b/cmds/idmap2/include/idmap2/FabricatedOverlay.h
index 9f57710..a29fa8f 100644
--- a/cmds/idmap2/include/idmap2/FabricatedOverlay.h
+++ b/cmds/idmap2/include/idmap2/FabricatedOverlay.h
@@ -19,9 +19,10 @@
#include <libidmap2/proto/fabricated_v1.pb.h>
-#include <iostream>
+#include <istream>
#include <map>
#include <memory>
+#include <ostream>
#include <string>
#include <unordered_map>
#include <vector>
diff --git a/cmds/idmap2/include/idmap2/FileUtils.h b/cmds/idmap2/include/idmap2/FileUtils.h
index bc0bb47..3e99981 100644
--- a/cmds/idmap2/include/idmap2/FileUtils.h
+++ b/cmds/idmap2/include/idmap2/FileUtils.h
@@ -19,12 +19,12 @@
#include <sys/types.h>
-#include <random>
#include <string>
+#include <string_view>
namespace android::idmap2::utils {
-constexpr const char* kIdmapCacheDir = "/data/resource-cache";
+constexpr std::string_view kIdmapCacheDir = "/data/resource-cache";
constexpr const mode_t kIdmapFilePermissionMask = 0133; // u=rw,g=r,o=r
bool UidHasWriteAccessToPath(uid_t uid, const std::string& path);
diff --git a/cmds/idmap2/include/idmap2/Idmap.h b/cmds/idmap2/include/idmap2/Idmap.h
index 03e714a..e86f814 100644
--- a/cmds/idmap2/include/idmap2/Idmap.h
+++ b/cmds/idmap2/include/idmap2/Idmap.h
@@ -71,9 +71,10 @@
#ifndef IDMAP2_INCLUDE_IDMAP2_IDMAP_H_
#define IDMAP2_INCLUDE_IDMAP2_IDMAP_H_
-#include <iostream>
+#include <istream>
#include <memory>
#include <string>
+#include <string_view>
#include <vector>
#include "android-base/macros.h"
@@ -272,8 +273,8 @@
class Idmap {
public:
- static std::string CanonicalIdmapPathFor(const std::string& absolute_dir,
- const std::string& absolute_apk_path);
+ static std::string CanonicalIdmapPathFor(std::string_view absolute_dir,
+ std::string_view absolute_apk_path);
static Result<std::unique_ptr<const Idmap>> FromBinaryStream(std::istream& stream);
diff --git a/cmds/idmap2/include/idmap2/LogInfo.h b/cmds/idmap2/include/idmap2/LogInfo.h
index a6237e6..b576152 100644
--- a/cmds/idmap2/include/idmap2/LogInfo.h
+++ b/cmds/idmap2/include/idmap2/LogInfo.h
@@ -61,7 +61,7 @@
#ifdef __ANDROID__
LOG(WARNING) << msg.GetString();
#else
- std::cerr << "W " << msg.GetString() << std::endl;
+ std::cerr << "W " << msg.GetString() << '\n';
#endif
lines_.push_back("W " + msg.GetString());
}
diff --git a/cmds/idmap2/include/idmap2/PrettyPrintVisitor.h b/cmds/idmap2/include/idmap2/PrettyPrintVisitor.h
index 4464201..ed18d9c 100644
--- a/cmds/idmap2/include/idmap2/PrettyPrintVisitor.h
+++ b/cmds/idmap2/include/idmap2/PrettyPrintVisitor.h
@@ -17,7 +17,7 @@
#ifndef IDMAP2_INCLUDE_IDMAP2_PRETTYPRINTVISITOR_H_
#define IDMAP2_INCLUDE_IDMAP2_PRETTYPRINTVISITOR_H_
-#include <iostream>
+#include <ostream>
#include <memory>
#include "androidfw/AssetManager2.h"
diff --git a/cmds/idmap2/include/idmap2/RawPrintVisitor.h b/cmds/idmap2/include/idmap2/RawPrintVisitor.h
index ebd0d1e..849ba11 100644
--- a/cmds/idmap2/include/idmap2/RawPrintVisitor.h
+++ b/cmds/idmap2/include/idmap2/RawPrintVisitor.h
@@ -17,7 +17,7 @@
#ifndef IDMAP2_INCLUDE_IDMAP2_RAWPRINTVISITOR_H_
#define IDMAP2_INCLUDE_IDMAP2_RAWPRINTVISITOR_H_
-#include <iostream>
+#include <ostream>
#include <memory>
#include <string>
diff --git a/cmds/idmap2/include/idmap2/XmlParser.h b/cmds/idmap2/include/idmap2/XmlParser.h
index c968a5e..c93b067 100644
--- a/cmds/idmap2/include/idmap2/XmlParser.h
+++ b/cmds/idmap2/include/idmap2/XmlParser.h
@@ -17,8 +17,6 @@
#ifndef IDMAP2_INCLUDE_IDMAP2_XMLPARSER_H_
#define IDMAP2_INCLUDE_IDMAP2_XMLPARSER_H_
-#include <iostream>
-#include <map>
#include <memory>
#include <string>
diff --git a/cmds/idmap2/libidmap2/CommandLineOptions.cpp b/cmds/idmap2/libidmap2/CommandLineOptions.cpp
index 8129d99..888b3a5 100644
--- a/cmds/idmap2/libidmap2/CommandLineOptions.cpp
+++ b/cmds/idmap2/libidmap2/CommandLineOptions.cpp
@@ -19,8 +19,8 @@
#include <algorithm>
#include <cassert>
#include <iomanip>
-#include <iostream>
#include <memory>
+#include <ostream>
#include <set>
#include <sstream>
#include <string>
@@ -131,7 +131,7 @@
separator = true;
stream << opt << ": missing mandatory option";
}
- stream << std::endl;
+ stream << '\n';
Usage(stream);
return Error("%s", stream.str().c_str());
}
@@ -168,7 +168,7 @@
out << " [" << opt.name << " arg [..]]";
}
}
- out << std::endl << std::endl;
+ out << "\n\n";
for (const Option& opt : options_) {
out << std::left << std::setw(maxLength);
if (opt.argument) {
@@ -181,7 +181,7 @@
opt.count == Option::COUNT_OPTIONAL_ONCE_OR_MORE) {
out << " (can be provided multiple times)";
}
- out << std::endl;
+ out << '\n';
}
}
diff --git a/cmds/idmap2/libidmap2/FileUtils.cpp b/cmds/idmap2/libidmap2/FileUtils.cpp
index 98a4cea..bc5654a 100644
--- a/cmds/idmap2/libidmap2/FileUtils.cpp
+++ b/cmds/idmap2/libidmap2/FileUtils.cpp
@@ -16,11 +16,13 @@
#include "idmap2/FileUtils.h"
+#include <random>
#include <string>
+#include <string_view>
#include "android-base/file.h"
#include "android-base/macros.h"
-#include "android-base/stringprintf.h"
+#include "android-base/strings.h"
#include "private/android_filesystem_config.h"
namespace android::idmap2::utils {
@@ -33,9 +35,9 @@
return false;
}
- const std::string cache_subdir = base::StringPrintf("%s/", kIdmapCacheDir);
- if (canonical_path == kIdmapCacheDir ||
- canonical_path.compare(0, cache_subdir.size(), cache_subdir) == 0) {
+ if (base::StartsWith(canonical_path, kIdmapCacheDir) &&
+ (canonical_path.size() == kIdmapCacheDir.size() ||
+ canonical_path[kIdmapCacheDir.size()] == '/')) {
// limit access to /data/resource-cache to root and system
return uid == AID_ROOT || uid == AID_SYSTEM;
}
@@ -47,17 +49,17 @@
}
#endif
-std::string RandomStringForPath(const size_t length) {
- constexpr char kChars[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
- constexpr size_t kCharLastIndex = sizeof(kChars) - 1;
+std::string RandomStringForPath(size_t length) {
+ constexpr std::string_view kChars =
+ "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
std::string out_rand;
- out_rand.reserve(length);
+ out_rand.resize(length);
- std::random_device rd;
- std::uniform_int_distribution<int> dist(0, kCharLastIndex);
+ static thread_local std::random_device rd;
+ std::uniform_int_distribution<int> dist(0, kChars.size() - 1);
for (size_t i = 0; i < length; i++) {
- out_rand[i] = kChars[dist(rd) % (kCharLastIndex)];
+ out_rand[i] = kChars[dist(rd)];
}
return out_rand;
}
diff --git a/cmds/idmap2/libidmap2/Idmap.cpp b/cmds/idmap2/libidmap2/Idmap.cpp
index 7c0b937..12d9dd9 100644
--- a/cmds/idmap2/libidmap2/Idmap.cpp
+++ b/cmds/idmap2/libidmap2/Idmap.cpp
@@ -18,13 +18,14 @@
#include <algorithm>
#include <cassert>
-#include <iostream>
+#include <istream>
#include <iterator>
#include <limits>
#include <memory>
#include <string>
#include <utility>
+#include "android-base/format.h"
#include "android-base/macros.h"
#include "androidfw/AssetManager2.h"
#include "idmap2/ResourceMapping.h"
@@ -80,7 +81,7 @@
if (padding_size != 0 && !stream.seekg(padding_size, std::ios_base::cur)) {
return false;
}
- *out = buf;
+ *out = std::move(buf);
return true;
}
@@ -279,13 +280,13 @@
return std::move(data);
}
-std::string Idmap::CanonicalIdmapPathFor(const std::string& absolute_dir,
- const std::string& absolute_apk_path) {
+std::string Idmap::CanonicalIdmapPathFor(std::string_view absolute_dir,
+ std::string_view absolute_apk_path) {
assert(absolute_dir.size() > 0 && absolute_dir[0] == "/");
assert(absolute_apk_path.size() > 0 && absolute_apk_path[0] == "/");
- std::string copy(++absolute_apk_path.cbegin(), absolute_apk_path.cend());
+ std::string copy(absolute_apk_path.begin() + 1, absolute_apk_path.end());
replace(copy.begin(), copy.end(), '/', '@');
- return absolute_dir + "/" + copy + "@idmap";
+ return fmt::format("{}/{}@idmap", absolute_dir, copy);
}
Result<std::unique_ptr<const Idmap>> Idmap::FromBinaryStream(std::istream& stream) {
@@ -332,7 +333,7 @@
values[cd] = value;
inline_value_count++;
}
- data->target_inline_entries_.push_back({mapping.first, values});
+ data->target_inline_entries_.push_back({mapping.first, std::move(values)});
}
}
diff --git a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
index a44fa75..eb94582 100644
--- a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
+++ b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
@@ -34,21 +34,21 @@
}
void PrettyPrintVisitor::visit(const IdmapHeader& header) {
- stream_ << "Paths:" << std::endl
- << TAB "target path : " << header.GetTargetPath() << std::endl
- << TAB "overlay path : " << header.GetOverlayPath() << std::endl;
+ stream_ << "Paths:" << '\n'
+ << TAB "target path : " << header.GetTargetPath() << '\n'
+ << TAB "overlay path : " << header.GetOverlayPath() << '\n';
if (!header.GetOverlayName().empty()) {
- stream_ << "Overlay name: " << header.GetOverlayName() << std::endl;
+ stream_ << "Overlay name: " << header.GetOverlayName() << '\n';
}
const std::string& debug = header.GetDebugInfo();
if (!debug.empty()) {
std::istringstream debug_stream(debug);
std::string line;
- stream_ << "Debug info:" << std::endl;
+ stream_ << "Debug info:" << '\n';
while (std::getline(debug_stream, line)) {
- stream_ << TAB << line << std::endl;
+ stream_ << TAB << line << '\n';
}
}
@@ -59,7 +59,7 @@
overlay_ = std::move(*overlay);
}
- stream_ << "Mapping:" << std::endl;
+ stream_ << "Mapping:" << '\n';
}
void PrettyPrintVisitor::visit(const IdmapData::Header& header ATTRIBUTE_UNUSED) {
@@ -90,7 +90,7 @@
<< base::StringPrintf("0x%08x -> 0x%08x (%s -> %s)", target_entry.target_id,
target_entry.overlay_id, target_name.c_str(),
overlay_name.c_str())
- << std::endl;
+ << '\n';
}
for (auto& target_entry : data.GetTargetInlineEntries()) {
@@ -114,7 +114,7 @@
}
}
- stream_ << " (" << target_name << ")" << std::endl;
+ stream_ << " (" << target_name << ")" << '\n';
}
}
diff --git a/cmds/idmap2/libidmap2/RawPrintVisitor.cpp b/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
index 3531cd7..174d85c 100644
--- a/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
+++ b/cmds/idmap2/libidmap2/RawPrintVisitor.cpp
@@ -161,7 +161,7 @@
va_end(ap);
stream_ << base::StringPrintf("%08zx: %02x", offset_, value) << " " << comment
- << std::endl;
+ << '\n';
offset_ += sizeof(uint8_t);
}
@@ -173,7 +173,7 @@
base::StringAppendV(&comment, fmt, ap);
va_end(ap);
- stream_ << base::StringPrintf("%08zx: %04x", offset_, value) << " " << comment << std::endl;
+ stream_ << base::StringPrintf("%08zx: %04x", offset_, value) << " " << comment << '\n';
offset_ += sizeof(uint16_t);
}
@@ -185,7 +185,7 @@
base::StringAppendV(&comment, fmt, ap);
va_end(ap);
- stream_ << base::StringPrintf("%08zx: %08x", offset_, value) << " " << comment << std::endl;
+ stream_ << base::StringPrintf("%08zx: %08x", offset_, value) << " " << comment << '\n';
offset_ += sizeof(uint32_t);
}
@@ -198,7 +198,7 @@
va_end(ap);
stream_ << base::StringPrintf("%08zx: %08x", offset_, (uint32_t)value.size()) << " " << comment
- << " size" << std::endl;
+ << " size" << '\n';
offset_ += sizeof(uint32_t);
stream_ << base::StringPrintf("%08zx: ", offset_) << "........ " << comment;
@@ -207,7 +207,7 @@
if (print_value) {
stream_ << ": " << value;
}
- stream_ << std::endl;
+ stream_ << '\n';
}
void RawPrintVisitor::align() {
diff --git a/cmds/idmap2/libidmap2/XmlParser.cpp b/cmds/idmap2/libidmap2/XmlParser.cpp
index 70822c8..766ca56 100644
--- a/cmds/idmap2/libidmap2/XmlParser.cpp
+++ b/cmds/idmap2/libidmap2/XmlParser.cpp
@@ -16,8 +16,6 @@
#include "idmap2/XmlParser.h"
-#include <iostream>
-#include <map>
#include <memory>
#include <string>
#include <utility>
diff --git a/cmds/idmap2/tests/FileUtilsTests.cpp b/cmds/idmap2/tests/FileUtilsTests.cpp
index 5750ca1..b160e8e 100644
--- a/cmds/idmap2/tests/FileUtilsTests.cpp
+++ b/cmds/idmap2/tests/FileUtilsTests.cpp
@@ -27,8 +27,9 @@
#ifdef __ANDROID__
TEST(FileUtilsTests, UidHasWriteAccessToPath) {
constexpr const char* tmp_path = "/data/local/tmp/test@idmap";
- const std::string cache_path(base::StringPrintf("%s/test@idmap", kIdmapCacheDir));
- const std::string sneaky_cache_path(base::StringPrintf("/data/../%s/test@idmap", kIdmapCacheDir));
+ const std::string cache_path(base::StringPrintf("%s/test@idmap", kIdmapCacheDir.data()));
+ const std::string sneaky_cache_path(
+ base::StringPrintf("/data/../%s/test@idmap", kIdmapCacheDir.data()));
ASSERT_TRUE(UidHasWriteAccessToPath(AID_ROOT, tmp_path));
ASSERT_TRUE(UidHasWriteAccessToPath(AID_ROOT, cache_path));
diff --git a/cmds/idmap2/tests/IdmapTests.cpp b/cmds/idmap2/tests/IdmapTests.cpp
index b473f26..f6e48ba 100644
--- a/cmds/idmap2/tests/IdmapTests.cpp
+++ b/cmds/idmap2/tests/IdmapTests.cpp
@@ -613,19 +613,19 @@
}
void visit(const Idmap& idmap ATTRIBUTE_UNUSED) override {
- stream_ << "TestVisitor::visit(Idmap)" << std::endl;
+ stream_ << "TestVisitor::visit(Idmap)" << '\n';
}
void visit(const IdmapHeader& idmap ATTRIBUTE_UNUSED) override {
- stream_ << "TestVisitor::visit(IdmapHeader)" << std::endl;
+ stream_ << "TestVisitor::visit(IdmapHeader)" << '\n';
}
void visit(const IdmapData& idmap ATTRIBUTE_UNUSED) override {
- stream_ << "TestVisitor::visit(IdmapData)" << std::endl;
+ stream_ << "TestVisitor::visit(IdmapData)" << '\n';
}
void visit(const IdmapData::Header& idmap ATTRIBUTE_UNUSED) override {
- stream_ << "TestVisitor::visit(IdmapData::Header)" << std::endl;
+ stream_ << "TestVisitor::visit(IdmapData::Header)" << '\n';
}
private: