commit 971e5f0e1d8cbefc71caddb18c0e993f49e5223d
author Jooyung Han <jooyung@google.com> Tue Jun 11 13:38:01 2019 +0900
committer Jooyung Han <jooyung@google.com> Tue Jun 11 13:38:01 2019 +0900
tree 218261cc2ac59077c3889f1d1fd83e32a878f725
parent 71ab050129930cd24163362a19a80cfd34e6fc91

fix: passing ResultError to Error thru operator<<

It is typical to pass error to callers like following;

    if (!result) {
      return Error() << result.error();
    }

To transfer errno(or ResultError#code()), Error defines a specialization
operator<<(const ResultError&).

This change fixes so that ResultError is properly handled

Bug: 132145659
Test: atest libbase_test
Change-Id: Ib35457da2d4b923d8e652c54ac510a75546cf918

base/include/android-base/result.h

diff --git a/base/include/android-base/result.h b/base/include/android-base/result.h
index 897c48f..4a8e1ef 100644
--- a/base/include/android-base/result.h
+++ b/base/include/android-base/result.h
@@ -81,8 +81,7 @@
 
 struct ResultError {
   template <typename T>
-  ResultError(T&& message, int code)
-      : message_(std::forward<T>(message)), code_(code) {}
+  ResultError(T&& message, int code) : message_(std::forward<T>(message)), code_(code) {}
 
   template <typename T>
   operator android::base::expected<T, ResultError>() {
@@ -122,18 +121,16 @@
 
   template <typename T>
   Error& operator<<(T&& t) {
+    if constexpr (std::is_same_v<std::remove_cv_t<std::remove_reference_t<T>>, ResultError>) {
+      errno_ = t.code();
+      return (*this) << t.message();
+    }
     int saved = errno;
     ss_ << t;
     errno = saved;
     return *this;
   }
 
-  Error& operator<<(const ResultError& result_error) {
-    (*this) << result_error.message();
-    errno_ = result_error.code();
-    return *this;
-  }
-
   const std::string str() const {
     std::string str = ss_.str();
     if (append_errno_) {

base/result_test.cpp

diff --git a/base/result_test.cpp b/base/result_test.cpp
index 72f97f4..e864b97 100644
--- a/base/result_test.cpp
+++ b/base/result_test.cpp
@@ -143,8 +143,8 @@
   ASSERT_FALSE(result2);
   ASSERT_FALSE(result2.has_value());
 
-  EXPECT_EQ(0, result.error().code());
-  EXPECT_EQ(error_text, result.error().message());
+  EXPECT_EQ(0, result2.error().code());
+  EXPECT_EQ(error_text, result2.error().message());
 }
 
 TEST(result, result_error_through_ostream) {
@@ -159,8 +159,8 @@
   ASSERT_FALSE(result2);
   ASSERT_FALSE(result2.has_value());
 
-  EXPECT_EQ(0, result.error().code());
-  EXPECT_EQ(error_text, result.error().message());
+  EXPECT_EQ(0, result2.error().code());
+  EXPECT_EQ(error_text, result2.error().message());
 }
 
 TEST(result, result_errno_error_through_ostream) {
@@ -179,8 +179,8 @@
   ASSERT_FALSE(result2);
   ASSERT_FALSE(result2.has_value());
 
-  EXPECT_EQ(test_errno, result.error().code());
-  EXPECT_EQ(error_text + ": " + strerror(test_errno), result.error().message());
+  EXPECT_EQ(test_errno, result2.error().code());
+  EXPECT_EQ(error_text + ": " + strerror(test_errno), result2.error().message());
 }
 
 TEST(result, constructor_forwarding) {