commit ce42490bb5b499acbf0cc6738e4481e077a6d31a
author Mårten Kongstad <marten.kongstad@sony.com> Fri Mar 01 08:35:37 2019 +0100
committer Todd Kennedy <toddke@google.com> Wed Mar 20 13:45:14 2019 -0700
tree 60e54956ac4257ee03b5b4176e54346fe6a6b140
parent 3be2db9bbf33f56efb92c97c0a91ad8ab670f0e0

idmap2: move Idmap.h to Result

Change the signatures of Idmap::FromApkAssets and
Idmap::FromBinaryStream from

  std::unique_ptr<const Idmap> func(..., std::ostream& out_error);

to

  Result<std::unique_ptr<const Idmap>> func(...);

The returned pointer is still a unique pointer to ensure the dynamically
allocated memory is automatically released when no longer used. This
means that using the returned value of either function requires one of
two patterns:

  const auto idmap = func(...);
  if (!idmap) {
    return Error(...);
  }
  (*idmap)->accept(...);

or

  auto result = func(...);
  if (!result) {
    return Error(...);
  }
  const auto idmap = std::move(*result);
  idmap->accept(...);

Note that in the second example, result must be non-const or
the call to std::move(*result) will not compile.

With this change, the entire idmap2 project has been converted to use
Result.

Test: make idmap2_tests
Change-Id: I533f4e03b99645523d94dd5f446ad76fb435f661

cmds/idmap2/tests/IdmapTests.cpp

diff --git a/cmds/idmap2/tests/IdmapTests.cpp b/cmds/idmap2/tests/IdmapTests.cpp
index c20ae7b..621f503 100644
--- a/cmds/idmap2/tests/IdmapTests.cpp
+++ b/cmds/idmap2/tests/IdmapTests.cpp
@@ -20,6 +20,7 @@
 #include <memory>
 #include <sstream>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "gmock/gmock.h"
@@ -127,9 +128,9 @@
   std::string raw(reinterpret_cast<const char*>(idmap_raw_data), idmap_raw_data_len);
   std::istringstream stream(raw);
 
-  std::stringstream error;
-  std::unique_ptr<const Idmap> idmap = Idmap::FromBinaryStream(stream, error);
-  ASSERT_THAT(idmap, NotNull());
+  auto result = Idmap::FromBinaryStream(stream);
+  ASSERT_TRUE(result);
+  const auto idmap = std::move(*result);
 
   ASSERT_THAT(idmap->GetHeader(), NotNull());
   ASSERT_EQ(idmap->GetHeader()->GetMagic(), 0x504d4449U);
@@ -168,9 +169,8 @@
                   10);  // data too small
   std::istringstream stream(raw);
 
-  std::stringstream error;
-  std::unique_ptr<const Idmap> idmap = Idmap::FromBinaryStream(stream, error);
-  ASSERT_THAT(idmap, IsNull());
+  const auto result = Idmap::FromBinaryStream(stream);
+  ASSERT_FALSE(result);
 }
 
 void CreateIdmap(const StringPiece& target_apk_path, const StringPiece& overlay_apk_path,
@@ -182,10 +182,10 @@
   std::unique_ptr<const ApkAssets> overlay_apk = ApkAssets::Load(overlay_apk_path.to_string());
   ASSERT_THAT(overlay_apk, NotNull());
 
-  std::stringstream error;
-  *out_idmap =
+  auto result =
       Idmap::FromApkAssets(target_apk_path.to_string(), *target_apk, overlay_apk_path.to_string(),
-                           *overlay_apk, fulfilled_policies, enforce_overlayable, error);
+                           *overlay_apk, fulfilled_policies, enforce_overlayable);
+  *out_idmap = result ? std::move(*result) : nullptr;
 }
 
 TEST(IdmapTests, CreateIdmapFromApkAssets) {
@@ -471,11 +471,10 @@
   std::unique_ptr<const ApkAssets> overlay_apk = ApkAssets::Load(overlay_apk_path);
   ASSERT_THAT(overlay_apk, NotNull());
 
-  std::stringstream error;
-  std::unique_ptr<const Idmap> idmap =
+  const auto result =
       Idmap::FromApkAssets(target_apk_path, *target_apk, overlay_apk_path, *overlay_apk,
-                           PolicyFlags::POLICY_PUBLIC, /* enforce_overlayable */ true, error);
-  ASSERT_THAT(idmap, IsNull());
+                           PolicyFlags::POLICY_PUBLIC, /* enforce_overlayable */ true);
+  ASSERT_FALSE(result);
 }
 
 TEST(IdmapTests, IdmapHeaderIsUpToDate) {
@@ -489,11 +488,11 @@
   std::unique_ptr<const ApkAssets> overlay_apk = ApkAssets::Load(overlay_apk_path);
   ASSERT_THAT(overlay_apk, NotNull());
 
-  std::stringstream error;
-  std::unique_ptr<const Idmap> idmap = Idmap::FromApkAssets(
-      target_apk_path, *target_apk, overlay_apk_path, *overlay_apk, PolicyFlags::POLICY_PUBLIC,
-      /* enforce_overlayable */ true, error);
-  ASSERT_THAT(idmap, NotNull());
+  auto result = Idmap::FromApkAssets(target_apk_path, *target_apk, overlay_apk_path, *overlay_apk,
+                                     PolicyFlags::POLICY_PUBLIC,
+                                     /* enforce_overlayable */ true);
+  ASSERT_TRUE(result);
+  const auto idmap = std::move(*result);
 
   std::stringstream stream;
   BinaryStreamVisitor visitor(stream);
@@ -609,13 +608,12 @@
   std::string raw(reinterpret_cast<const char*>(idmap_raw_data), idmap_raw_data_len);
   std::istringstream stream(raw);
 
-  std::stringstream error;
-  std::unique_ptr<const Idmap> idmap = Idmap::FromBinaryStream(stream, error);
-  ASSERT_THAT(idmap, NotNull());
+  const auto idmap = Idmap::FromBinaryStream(stream);
+  ASSERT_TRUE(idmap);
 
   std::stringstream test_stream;
   TestVisitor visitor(test_stream);
-  idmap->accept(&visitor);
+  (*idmap)->accept(&visitor);
 
   ASSERT_EQ(test_stream.str(),
             "TestVisitor::visit(Idmap)\n"