Merge changes from topic "sideload_logging" into rvc-dev

* changes:
  sideload: fix duplicated logging
  Revert "Setup android-base logging in sideload."

5 files changed, 54 insertions, 75 deletions

diff --git a/Android.bp b/Android.bp
index 07eee639..3287b7b4 100644
--- a/Android.bp
+++ b/Android.bp
@@ -356,10 +356,10 @@ cc_binary {
 
     srcs: [
         "hardware_android.cc",
+        "logging_android.cc",
         "metrics_reporter_stub.cc",
         "metrics_utils.cc",
         "network_selector_stub.cc",
-        "sideload_logging_android.cc",
         "sideload_main.cc",
         "update_attempter_android.cc",
         "update_boot_flags_action.cc",
diff --git a/logging_android.cc b/logging_android.cc
index d5aac6df..88b068bc 100644
--- a/logging_android.cc
+++ b/logging_android.cc
@@ -39,6 +39,12 @@
 
 using std::string;
 
+#ifdef _UE_SIDELOAD
+constexpr bool kSideload = true;
+#else
+constexpr bool kSideload = false;
+#endif
+
 namespace chromeos_update_engine {
 namespace {
 
@@ -141,13 +147,11 @@ class FileLogger {
       return;
     }
 
-    // libchrome add a newline character to |message|. Strip it.
-    std::string_view message_no_newline =
+    std::string_view message_str =
         log_message->message != nullptr ? log_message->message : "";
-    ignore_result(android::base::ConsumeSuffix(&message_no_newline, "\n"));
 
     WriteToFd(GetPrefix(log_message));
-    WriteToFd(message_no_newline);
+    WriteToFd(message_str);
     WriteToFd("\n");
   }
 
@@ -187,7 +191,13 @@ class CombinedLogger {
  public:
   CombinedLogger(bool log_to_system, bool log_to_file) {
     if (log_to_system) {
-      loggers_.push_back(__android_log_logd_logger);
+      if (kSideload) {
+        // No logd in sideload. Use stdout.
+        // recovery has already redirected stdio properly.
+        loggers_.push_back(__android_log_stderr_logger);
+      } else {
+        loggers_.push_back(__android_log_logd_logger);
+      }
     }
     if (log_to_file) {
       loggers_.push_back(std::move(FileLogger(SetupLogFile(kSystemLogsRoot))));
@@ -203,6 +213,39 @@ class CombinedLogger {
   std::vector<LoggerFunction> loggers_;
 };
 
+// Redirect all libchrome logs to liblog using our custom handler that does
+// not call __android_log_write and explicitly write to stderr at the same
+// time. The preset CombinedLogger already writes to stderr properly.
+bool RedirectToLiblog(int severity,
+                      const char* file,
+                      int line,
+                      size_t message_start,
+                      const std::string& str_newline) {
+  android_LogPriority priority =
+      (severity < 0) ? ANDROID_LOG_VERBOSE : ANDROID_LOG_UNKNOWN;
+  switch (severity) {
+    case logging::LOG_INFO:
+      priority = ANDROID_LOG_INFO;
+      break;
+    case logging::LOG_WARNING:
+      priority = ANDROID_LOG_WARN;
+      break;
+    case logging::LOG_ERROR:
+      priority = ANDROID_LOG_ERROR;
+      break;
+    case logging::LOG_FATAL:
+      priority = ANDROID_LOG_FATAL;
+      break;
+  }
+  std::string_view sv = str_newline;
+  ignore_result(android::base::ConsumeSuffix(&sv, "\n"));
+  std::string str(sv.data(), sv.size());
+  // This will eventually be redirected to CombinedLogger.
+  // |tag| is ignored by CombinedLogger, so just leave it empty.
+  __android_log_write(priority, "" /* tag */, str.c_str());
+  return true;
+}
+
 }  // namespace
 
 void SetupLogging(bool log_to_system, bool log_to_file) {
@@ -219,14 +262,15 @@ void SetupLogging(bool log_to_system, bool log_to_file) {
   // libchrome logging should not log to file.
   logging::LoggingSettings log_settings;
   log_settings.lock_log = logging::DONT_LOCK_LOG_FILE;
-  log_settings.logging_dest = static_cast<logging::LoggingDestination>(
-      logging::LOG_TO_SYSTEM_DEBUG_LOG);
+  log_settings.logging_dest =
+      static_cast<logging::LoggingDestination>(logging::LOG_NONE);
   log_settings.log_file = nullptr;
   logging::InitLogging(log_settings);
   logging::SetLogItems(false /* enable_process_id */,
                        false /* enable_thread_id */,
                        false /* enable_timestamp */,
                        false /* enable_tickcount */);
+  logging::SetLogMessageHandler(&RedirectToLiblog);
 }
 
 }  // namespace chromeos_update_engine
diff --git a/sideload_logging_android.cc b/sideload_logging_android.cc
deleted file mode 100644
index f82259f3..00000000
--- a/sideload_logging_android.cc
+++ /dev/null
@@ -1,27 +0,0 @@
-//
-// Copyright (C) 2019 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//      http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-#include "update_engine/sideload_logging_android.h"
-
-#include <android-base/logging.h>
-
-namespace chromeos_update_engine {
-
-void SetupAndroidLogging(char* argv[]) {
-  android::base::InitLogging(argv, android::base::StdioLogger);
-}
-
-}  // namespace chromeos_update_engine
diff --git a/sideload_logging_android.h b/sideload_logging_android.h
deleted file mode 100644
index 0bb87146..00000000
--- a/sideload_logging_android.h
+++ /dev/null
@@ -1,25 +0,0 @@
-//
-// Copyright (C) 2019 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//      http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-#pragma once
-
-namespace chromeos_update_engine {
-
-// Some depending modules uses logging functions from android-base.
-// Redirect android-base logging to stdio, which redirects to /tmp/recovery.log.
-void SetupAndroidLogging(char* argv[]);
-
-}  // namespace chromeos_update_engine
diff --git a/sideload_main.cc b/sideload_main.cc
index 29d6f2ce..27967cda 100644
--- a/sideload_main.cc
+++ b/sideload_main.cc
@@ -20,7 +20,6 @@
 #include <vector>
 
 #include <base/command_line.h>
-#include <base/logging.h>
 #include <base/strings/string_split.h>
 #include <base/strings/stringprintf.h>
 #include <brillo/asynchronous_signal_handler.h>
@@ -36,7 +35,7 @@
 #include "update_engine/common/subprocess.h"
 #include "update_engine/common/terminator.h"
 #include "update_engine/common/utils.h"
-#include "update_engine/sideload_logging_android.h"
+#include "update_engine/logging.h"
 #include "update_engine/update_attempter_android.h"
 
 using std::string;
@@ -47,17 +46,6 @@ using update_engine::UpdateStatus;
 namespace chromeos_update_engine {
 namespace {
 
-void SetupLogging() {
-  string log_file;
-  logging::LoggingSettings log_settings;
-  log_settings.lock_log = logging::DONT_LOCK_LOG_FILE;
-  log_settings.delete_old = logging::APPEND_TO_OLD_LOG_FILE;
-  log_settings.log_file = nullptr;
-  log_settings.logging_dest = logging::LOG_TO_SYSTEM_DEBUG_LOG;
-
-  logging::InitLogging(log_settings);
-}
-
 class SideloadDaemonState : public DaemonStateInterface,
                             public ServiceObserverInterface {
  public:
@@ -196,8 +184,7 @@ int main(int argc, char** argv) {
   DEFINE_int64(status_fd, -1, "A file descriptor to notify the update status.");
 
   chromeos_update_engine::Terminator::Init();
-  chromeos_update_engine::SetupLogging();
-  chromeos_update_engine::SetupAndroidLogging(argv);
+  chromeos_update_engine::SetupLogging(true /* stderr */, false /* file */);
   brillo::FlagHelper::Init(argc, argv, "Update Engine Sideload");
 
   LOG(INFO) << "Update Engine Sideloading starting";