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: