From f8e6bf6d819ae965deed6469bf85a8ea60488741 Mon Sep 17 00:00:00 2001
From: Tom Cherry <tomcherry@google.com>
Date: Tue, 4 Feb 2020 15:18:29 -0800
Subject: [PATCH] Check Errorf()/ErrnoErrorf() format string at compile time

fmtlib provides compile time checking of format strings that we're not
currently using.  This change makes Errorf() and ErrnoErrorf() into
macros such that we can take advantage of this capability.

Test: build successfully normally
Test: fail the build if using an invalid format string
Change-Id: Icb8ba8cb973bbd1fa4755a62e7598bdbb0113757
---
 base/include/android-base/result.h | 24 ++++++++++++++++--------
 base/result_test.cpp               |  2 +-
 init/result.h                      |  2 --
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/base/include/android-base/result.h b/base/include/android-base/result.h
index b6d26e7b9..e5d90c860 100644
--- a/base/include/android-base/result.h
+++ b/base/include/android-base/result.h
@@ -156,11 +156,11 @@ class Error {
   Error& operator=(const Error&) = delete;
   Error& operator=(Error&&) = delete;
 
-  template <typename... Args>
-  friend Error Errorf(const char* fmt, const Args&... args);
+  template <typename T, typename... Args>
+  friend Error ErrorfImpl(const T&& fmt, const Args&... args);
 
-  template <typename... Args>
-  friend Error ErrnoErrorf(const char* fmt, const Args&... args);
+  template <typename T, typename... Args>
+  friend Error ErrnoErrorfImpl(const T&& fmt, const Args&... args);
 
  private:
   Error(bool append_errno, int errno_to_append, const std::string& message)
@@ -191,16 +191,24 @@ inline int ErrorCode(int code, T&& t, const Args&... args) {
   return ErrorCode(code, args...);
 }
 
-template <typename... Args>
-inline Error Errorf(const char* fmt, const Args&... args) {
+// TODO(tomcherry): Remove this once we've removed all `using android::base::Errorf` and `using
+// android::base::ErrnoErrorf` lines.
+enum Errorf {};
+enum ErrnoErrorf {};
+
+template <typename T, typename... Args>
+inline Error ErrorfImpl(const T&& fmt, const Args&... args) {
   return Error(false, ErrorCode(0, args...), fmt::format(fmt, args...));
 }
 
-template <typename... Args>
-inline Error ErrnoErrorf(const char* fmt, const Args&... args) {
+template <typename T, typename... Args>
+inline Error ErrnoErrorfImpl(const T&& fmt, const Args&... args) {
   return Error(true, errno, fmt::format(fmt, args...));
 }
 
+#define Errorf(fmt, ...) android::base::ErrorfImpl(FMT_STRING(fmt), ##__VA_ARGS__)
+#define ErrnoErrorf(fmt, ...) android::base::ErrnoErrorfImpl(FMT_STRING(fmt), ##__VA_ARGS__)
+
 template <typename T>
 using Result = android::base::expected<T, ResultError>;
 
diff --git a/base/result_test.cpp b/base/result_test.cpp
index 2ee4057eb..0ea62e8cb 100644
--- a/base/result_test.cpp
+++ b/base/result_test.cpp
@@ -362,7 +362,7 @@ TEST(result, error_with_fmt) {
   result = Errorf("{} {}!", std::string("hello"), std::string("world"));
   EXPECT_EQ("hello world!", result.error().message());
 
-  result = Errorf("{h} {w}!", fmt::arg("w", "world"), fmt::arg("h", "hello"));
+  result = Errorf("{1} {0}!", "world", "hello");
   EXPECT_EQ("hello world!", result.error().message());
 
   result = Errorf("hello world!");
diff --git a/init/result.h b/init/result.h
index b70dd1b44..8c1f91e03 100644
--- a/init/result.h
+++ b/init/result.h
@@ -22,8 +22,6 @@
 #include <android-base/result.h>
 
 using android::base::ErrnoError;
-using android::base::ErrnoErrorf;
 using android::base::Error;
-using android::base::Errorf;
 using android::base::Result;
 using android::base::ResultError;