crash-reporter: Use standard logging and new libchromeos Process code
Change-Id: Ie085d61d4d79c0df38e51debaa044d701a860c89
BUG=none
TEST=unit tests / UserCrash / CrashSender
Review URL: http://codereview.chromium.org/6517001
diff --git a/crash_reporter/Makefile b/crash_reporter/Makefile
index 0523c19..b649bea 100644
--- a/crash_reporter/Makefile
+++ b/crash_reporter/Makefile
@@ -7,10 +7,9 @@
CRASH_OBJS = \
crash_collector.o \
kernel_collector.o \
- system_logging.o \
unclean_shutdown_collector.o \
user_collector.o
-TEST_OBJS = $(CRASH_OBJS) system_logging_mock.o
+TEST_OBJS = $(CRASH_OBJS)
TEST_BINS = \
crash_collector_test \
kernel_collector_test \
@@ -20,7 +19,7 @@
LDCONFIG = $(shell $(PKG_CONFIG) --libs libpcrecpp)
# -lglib-2.0 is needed by libbase.a now.
-COMMON_LIBS = -lbase -lpthread -lglib-2.0 -lgflags -lrt $(LDCONFIG)
+COMMON_LIBS = -lchromeos -lbase -lpthread -lglib-2.0 -lgflags -lrt $(LDCONFIG)
REPORTER_LIBS = $(COMMON_LIBS) -lmetrics
TEST_LIBS = $(COMMON_LIBS) -lgtest -lgmock
diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc
index afd72d1..9600b44 100644
--- a/crash_reporter/crash_collector.cc
+++ b/crash_reporter/crash_collector.cc
@@ -17,7 +17,7 @@
#include "base/file_util.h"
#include "base/logging.h"
#include "base/string_util.h"
-#include "crash-reporter/system_logging.h"
+#include "chromeos/process.h"
static const char kDefaultUserName[] = "chronos";
static const char kLsbRelease[] = "/etc/lsb-release";
@@ -54,15 +54,12 @@
void CrashCollector::Initialize(
CrashCollector::CountCrashFunction count_crash_function,
- CrashCollector::IsFeedbackAllowedFunction is_feedback_allowed_function,
- SystemLogging *logger) {
+ CrashCollector::IsFeedbackAllowedFunction is_feedback_allowed_function) {
CHECK(count_crash_function != NULL);
CHECK(is_feedback_allowed_function != NULL);
- CHECK(logger != NULL);
count_crash_function_ = count_crash_function;
is_feedback_allowed_function_ = is_feedback_allowed_function;
- logger_ = logger;
}
int CrashCollector::WriteNewFile(const FilePath &filename,
@@ -79,63 +76,6 @@
return rv;
}
-int CrashCollector::ForkExecAndPipe(std::vector<const char *> &arguments,
- const char *output_file) {
- // Copy off a writeable version of arguments.
- scoped_array<char*> argv(new char *[arguments.size() + 1]);
- int total_args_size = 0;
- for (size_t i = 0; i < arguments.size(); ++i) {
- if (arguments[i] == NULL) {
- logger_->LogError("Bad parameter");
- return -1;
- }
- total_args_size += strlen(arguments[i]) + 1;
- }
- scoped_array<char> buffer(new char[total_args_size]);
- char *buffer_pointer = &buffer[0];
-
- for (size_t i = 0; i < arguments.size(); ++i) {
- argv[i] = buffer_pointer;
- strcpy(buffer_pointer, arguments[i]);
- buffer_pointer += strlen(arguments[i]);
- *buffer_pointer = '\0';
- ++buffer_pointer;
- }
- argv[arguments.size()] = NULL;
-
- int pid = fork();
- if (pid < 0) {
- logger_->LogError("Fork failed: %d", errno);
- return -1;
- }
-
- if (pid == 0) {
- int output_handle = HANDLE_EINTR(
- open(output_file, O_CREAT | O_WRONLY | O_TRUNC | O_EXCL, 0666));
- if (output_handle < 0) {
- logger_->LogError("Could not create %s: %d", output_file, errno);
- // Avoid exit() to avoid atexit handlers from parent.
- _exit(127);
- }
- dup2(output_handle, 1);
- dup2(output_handle, 2);
- execv(argv[0], &argv[0]);
- logger_->LogError("Exec failed: %d", errno);
- _exit(127);
- }
-
- int status = 0;
- if (HANDLE_EINTR(waitpid(pid, &status, 0)) < 0) {
- logger_->LogError("Problem waiting for pid: %d", errno);
- return -1;
- }
- if (!WIFEXITED(status)) {
- logger_->LogError("Process did not exit normally: %d", status);
- return -1;
- }
- return WEXITSTATUS(status);
-}
-
std::string CrashCollector::Sanitize(const std::string &name) {
std::string result = name;
for (size_t i = 0; i < name.size(); ++i) {
@@ -199,7 +139,7 @@
if (getpwnam_r(name.c_str(), &passwd_storage, storage, sizeof(storage),
&passwd_result) != 0 || passwd_result == NULL) {
- logger_->LogError("Cannot find user named %s", name.c_str());
+ LOG(ERROR) << "Cannot find user named " << name;
return false;
}
@@ -225,7 +165,7 @@
if (!GetUserInfoFromName(kDefaultUserName,
&default_user_id,
&default_user_group)) {
- logger_->LogError("Could not find default user info");
+ LOG(ERROR) << "Could not find default user info";
return false;
}
mode_t directory_mode;
@@ -247,15 +187,15 @@
chown(crash_directory->value().c_str(),
directory_owner,
directory_group) < 0) {
- logger_->LogError("Unable to create appropriate crash directory");
+ LOG(ERROR) << "Unable to create appropriate crash directory";
return false;
}
umask(old_mask);
}
if (!file_util::PathExists(*crash_directory)) {
- logger_->LogError("Unable to create crash directory %s",
- crash_directory->value().c_str());
+ LOG(ERROR) << "Unable to create crash directory "
+ << crash_directory->value().c_str();
return false;
}
@@ -297,10 +237,9 @@
basenames.insert(basename);
if (basenames.size() >= static_cast<size_t>(kMaxCrashDirectorySize)) {
- logger_->LogWarning(
- "Crash directory %s already full with %d pending reports",
- crash_directory.value().c_str(),
- kMaxCrashDirectorySize);
+ LOG(WARNING) << "Crash directory " << crash_directory.value()
+ << " already full with " << kMaxCrashDirectorySize
+ << " pending reports";
full = true;
break;
}
@@ -350,24 +289,24 @@
const FilePath &output_file) {
std::map<std::string, std::string> log_commands;
if (!ReadKeyValueFile(config_path, ':', &log_commands)) {
- logger_->LogInfo("Unable to read log configuration file %s",
- config_path.value().c_str());
+ LOG(INFO) << "Unable to read log configuration file "
+ << config_path.value();
return false;
}
if (log_commands.find(exec_name) == log_commands.end())
return false;
- std::vector<const char *> command;
- command.push_back(kShellPath);
- command.push_back("-c");
+ chromeos::ProcessImpl diag_process;
+ diag_process.AddArg(kShellPath);
std::string shell_command = log_commands[exec_name];
- command.push_back(shell_command.c_str());
+ diag_process.AddStringOption("-c", shell_command);
+ diag_process.RedirectOutput(output_file.value());
- int fork_result = ForkExecAndPipe(command, output_file.value().c_str());
- if (fork_result != 0) {
- logger_->LogInfo("Running shell command %s failed with: %d",
- shell_command.c_str(), fork_result);
+ int result = diag_process.Run();
+ if (result != 0) {
+ LOG(INFO) << "Running shell command " << shell_command << "failed with: "
+ << result;
return false;
}
return true;
@@ -383,7 +322,7 @@
const std::string &payload_path) {
std::map<std::string, std::string> contents;
if (!ReadKeyValueFile(FilePath(std::string(lsb_release_)), '=', &contents)) {
- logger_->LogError("Problem parsing %s", lsb_release_);
+ LOG(ERROR) << "Problem parsing " << lsb_release_;
// Even though there was some failure, take as much as we could read.
}
std::string version("unknown");
@@ -407,6 +346,6 @@
// do not want to write with root access to a symlink that an attacker
// might have created.
if (WriteNewFile(meta_path, meta_data.c_str(), meta_data.size()) < 0) {
- logger_->LogError("Unable to write %s", meta_path.value().c_str());
+ LOG(ERROR) << "Unable to write " << meta_path.value();
}
}
diff --git a/crash_reporter/crash_collector.h b/crash_reporter/crash_collector.h
index f3fcbe5..3c09e2f 100644
--- a/crash_reporter/crash_collector.h
+++ b/crash_reporter/crash_collector.h
@@ -13,8 +13,6 @@
#include "base/file_path.h"
#include "gtest/gtest_prod.h" // for FRIEND_TEST
-class SystemLogging;
-
// User crash collector.
class CrashCollector {
public:
@@ -26,11 +24,9 @@
virtual ~CrashCollector();
// Initialize the crash collector for detection of crashes, given a
- // crash counting function, metrics collection enabled oracle, and
- // system logger facility.
+ // crash counting function, and metrics collection enabled oracle.
void Initialize(CountCrashFunction count_crash,
- IsFeedbackAllowedFunction is_metrics_allowed,
- SystemLogging *logger);
+ IsFeedbackAllowedFunction is_metrics_allowed);
protected:
friend class CrashCollectorTest;
@@ -66,9 +62,6 @@
// Otherwise returns the number of bytes written.
int WriteNewFile(const FilePath &filename, const char *data, int size);
- int ForkExecAndPipe(std::vector<const char *> &arguments,
- const char *output_file);
-
// Return a filename that has only [a-z0-1_] characters by mapping
// all others into '_'.
std::string Sanitize(const std::string &name);
@@ -140,7 +133,6 @@
CountCrashFunction count_crash_function_;
IsFeedbackAllowedFunction is_feedback_allowed_function_;
- SystemLogging *logger_;
std::string extra_metadata_;
const char *forced_crash_directory_;
const char *lsb_release_;
diff --git a/crash_reporter/crash_collector_test.cc b/crash_reporter/crash_collector_test.cc
index 8100a5f..d98e1af 100644
--- a/crash_reporter/crash_collector_test.cc
+++ b/crash_reporter/crash_collector_test.cc
@@ -6,9 +6,9 @@
#include "base/file_util.h"
#include "base/string_util.h"
+#include "chromeos/syslog_logging.h"
+#include "chromeos/test_helpers.h"
#include "crash-reporter/crash_collector.h"
-#include "crash-reporter/system_logging_mock.h"
-#include "crash-reporter/test_helpers.h"
#include "gflags/gflags.h"
#include "gtest/gtest.h"
@@ -18,6 +18,8 @@
static const char kBinEcho[] = "/bin/echo";
static const char kBinFalse[] = "/bin/false";
+using chromeos::FindLog;
+
void CountCrash() {
ADD_FAILURE();
}
@@ -31,10 +33,10 @@
public:
void SetUp() {
collector_.Initialize(CountCrash,
- IsMetrics,
- &logging_);
+ IsMetrics);
test_dir_ = FilePath("test");
file_util::CreateDirectory(test_dir_);
+ chromeos::ClearLog();
}
void TearDown() {
@@ -44,7 +46,6 @@
bool CheckHasCapacity();
protected:
- SystemLoggingMock logging_;
CrashCollector collector_;
FilePath test_dir_;
};
@@ -52,7 +53,6 @@
TEST_F(CrashCollectorTest, Initialize) {
ASSERT_TRUE(CountCrash == collector_.count_crash_function_);
ASSERT_TRUE(IsMetrics == collector_.is_feedback_allowed_function_);
- ASSERT_TRUE(&logging_ == collector_.logger_);
}
TEST_F(CrashCollectorTest, WriteNewFile) {
@@ -154,7 +154,7 @@
bool CrashCollectorTest::CheckHasCapacity() {
static const char kFullMessage[] = "Crash directory test already full";
bool has_capacity = collector_.CheckHasCapacity(test_dir_);
- bool has_message = (logging_.log().find(kFullMessage) != std::string::npos);
+ bool has_message = FindLog(kFullMessage);
EXPECT_EQ(has_message, !has_capacity);
return has_capacity;
}
@@ -298,24 +298,24 @@
symlink(kMetaFileBasename,
meta_symlink_path.value().c_str()));
ASSERT_TRUE(file_util::PathExists(meta_symlink_path));
- logging_.clear();
+ chromeos::ClearLog();
collector_.WriteCrashMetaData(meta_symlink_path,
"kernel",
payload_file.value());
- // Target metadata contents sould have stayed the same.
+ // Target metadata contents should have stayed the same.
contents.clear();
EXPECT_TRUE(file_util::ReadFileToString(meta_file, &contents));
EXPECT_EQ(kExpectedMeta, contents);
- EXPECT_NE(std::string::npos, logging_.log().find("Unable to write"));
+ EXPECT_TRUE(FindLog("Unable to write"));
// Test target of dangling symlink is not created.
file_util::Delete(meta_file, false);
ASSERT_FALSE(file_util::PathExists(meta_file));
- logging_.clear();
+ chromeos::ClearLog();
collector_.WriteCrashMetaData(meta_symlink_path, "kernel",
payload_file.value());
EXPECT_FALSE(file_util::PathExists(meta_file));
- EXPECT_NE(std::string::npos, logging_.log().find("Unable to write"));
+ EXPECT_TRUE(FindLog("Unable to write"));
}
TEST_F(CrashCollectorTest, GetLogContents) {
@@ -341,87 +341,7 @@
EXPECT_EQ("hello world\n", contents);
}
-class ForkExecAndPipeTest : public CrashCollectorTest {
- public:
- void SetUp() {
- CrashCollectorTest::SetUp();
- output_file_ = "test/fork_out";
- file_util::Delete(FilePath(output_file_), false);
- }
-
- void TearDown() {
- CrashCollectorTest::TearDown();
- }
-
- protected:
- std::vector<const char *> args_;
- const char *output_file_;
-};
-
-TEST_F(ForkExecAndPipeTest, Basic) {
- args_.push_back(kBinEcho);
- args_.push_back("hello world");
- EXPECT_EQ(0, collector_.ForkExecAndPipe(args_, output_file_));
- ExpectFileEquals("hello world\n", output_file_);
- EXPECT_EQ("", logging_.log());
-}
-
-TEST_F(ForkExecAndPipeTest, NonZeroReturnValue) {
- args_.push_back(kBinFalse);
- EXPECT_EQ(1, collector_.ForkExecAndPipe(args_, output_file_));
- ExpectFileEquals("", output_file_);
- EXPECT_EQ("", logging_.log());
-}
-
-TEST_F(ForkExecAndPipeTest, BadOutputFile) {
- EXPECT_EQ(127, collector_.ForkExecAndPipe(args_, "/bad/path"));
-}
-
-TEST_F(ForkExecAndPipeTest, ExistingOutputFile) {
- args_.push_back(kBinEcho);
- args_.push_back("hello world");
- EXPECT_FALSE(file_util::PathExists(FilePath(output_file_)));
- EXPECT_EQ(0, collector_.ForkExecAndPipe(args_, output_file_));
- EXPECT_TRUE(file_util::PathExists(FilePath(output_file_)));
- EXPECT_EQ(127, collector_.ForkExecAndPipe(args_, output_file_));
-}
-
-TEST_F(ForkExecAndPipeTest, BadExecutable) {
- args_.push_back("false");
- EXPECT_EQ(127, collector_.ForkExecAndPipe(args_, output_file_));
-}
-
-TEST_F(ForkExecAndPipeTest, StderrCaptured) {
- std::string contents;
- args_.push_back(kBinCp);
- EXPECT_EQ(1, collector_.ForkExecAndPipe(args_, output_file_));
- EXPECT_TRUE(file_util::ReadFileToString(FilePath(output_file_),
- &contents));
- EXPECT_NE(std::string::npos, contents.find("missing file operand"));
- EXPECT_EQ("", logging_.log());
-}
-
-TEST_F(ForkExecAndPipeTest, NULLParam) {
- args_.push_back(NULL);
- EXPECT_EQ(-1, collector_.ForkExecAndPipe(args_, output_file_));
- EXPECT_NE(std::string::npos,
- logging_.log().find("Bad parameter"));
-}
-
-TEST_F(ForkExecAndPipeTest, NoParams) {
- EXPECT_EQ(127, collector_.ForkExecAndPipe(args_, output_file_));
-}
-
-TEST_F(ForkExecAndPipeTest, SegFaultHandling) {
- args_.push_back(kBinBash);
- args_.push_back("-c");
- args_.push_back("kill -SEGV $$");
- EXPECT_EQ(-1, collector_.ForkExecAndPipe(args_, output_file_));
- EXPECT_NE(std::string::npos,
- logging_.log().find("Process did not exit normally"));
-}
-
int main(int argc, char **argv) {
- ::testing::InitGoogleTest(&argc, argv);
+ SetUpTests(&argc, argv, false);
return RUN_ALL_TESTS();
}
diff --git a/crash_reporter/crash_reporter.cc b/crash_reporter/crash_reporter.cc
index ab4c5ba..42998e9 100644
--- a/crash_reporter/crash_reporter.cc
+++ b/crash_reporter/crash_reporter.cc
@@ -2,14 +2,16 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <fcntl.h> // for open
+
#include <string>
#include "base/file_util.h"
#include "base/command_line.h"
#include "base/logging.h"
#include "base/string_util.h"
+#include "chromeos/syslog_logging.h"
#include "crash-reporter/kernel_collector.h"
-#include "crash-reporter/system_logging.h"
#include "crash-reporter/unclean_shutdown_collector.h"
#include "crash-reporter/user_collector.h"
#include "gflags/gflags.h"
@@ -41,7 +43,6 @@
};
static MetricsLibrary s_metrics_lib;
-static SystemLoggingImpl s_system_log;
static bool IsFeedbackAllowed() {
return s_metrics_lib.AreMetricsEnabled();
@@ -96,9 +97,7 @@
// to be restarted anyway.
int status = system(command.c_str());
- if (status != 0) {
- s_system_log.LogWarning("dbus-send running failed");
- }
+ LOG_IF(WARNING, status != 0) << "dbus-send running failed";
}
static int Initialize(KernelCollector *kernel_collector,
@@ -149,10 +148,10 @@
}
// Accumulate logs to help in diagnosing failures during user collection.
- s_system_log.set_accumulating(true);
+ chromeos::LogToString(true);
// Handle the crash, get the name of the process from procfs.
bool handled = user_collector->HandleCrash(FLAGS_user, NULL);
- s_system_log.set_accumulating(false);
+ chromeos::LogToString(false);
if (!handled)
return 1;
return 0;
@@ -178,31 +177,45 @@
return 0;
}
+// Ensure stdout, stdin, and stderr are open file descriptors. If
+// they are not, any code which writes to stderr/stdout may write out
+// to files opened during execution. In particular, when
+// crash_reporter is run by the kernel coredump pipe handler (via
+// kthread_create/kernel_execve), it will not have file table entries
+// 1 and 2 (stdout and stderr) populated. We populate them here.
+static void OpenStandardFileDescriptors() {
+ int new_fd = -1;
+ // We open /dev/null to fill in any of the standard [0, 2] file
+ // descriptors. We leave these open for the duration of the
+ // process. This works because open returns the lowest numbered
+ // invalid fd.
+ do {
+ new_fd = open("/dev/null", 0);
+ CHECK(new_fd >= 0) << "Unable to open /dev/null";
+ } while (new_fd >= 0 && new_fd <= 2);
+ close(new_fd);
+}
+
int main(int argc, char *argv[]) {
+ OpenStandardFileDescriptors();
google::ParseCommandLineFlags(&argc, &argv, true);
FilePath my_path(argv[0]);
file_util::AbsolutePath(&my_path);
s_metrics_lib.Init();
CommandLine::Init(argc, argv);
- logging::InitLogging(NULL,
- logging::LOG_ONLY_TO_SYSTEM_DEBUG_LOG,
- logging::DONT_LOCK_LOG_FILE,
- logging::DELETE_OLD_LOG_FILE);
- s_system_log.Initialize(my_path.BaseName().value().c_str());
+ chromeos::OpenLog(my_path.BaseName().value().c_str(), true);
+ chromeos::InitLog(chromeos::kLogToSyslog);
KernelCollector kernel_collector;
kernel_collector.Initialize(CountKernelCrash,
- IsFeedbackAllowed,
- &s_system_log);
+ IsFeedbackAllowed);
UserCollector user_collector;
user_collector.Initialize(CountUserCrash,
my_path.value(),
IsFeedbackAllowed,
- &s_system_log,
true); // generate_diagnostics
UncleanShutdownCollector unclean_shutdown_collector;
unclean_shutdown_collector.Initialize(CountUncleanShutdown,
- IsFeedbackAllowed,
- &s_system_log);
+ IsFeedbackAllowed);
if (FLAGS_init) {
return Initialize(&kernel_collector,
diff --git a/crash_reporter/kernel_collector.cc b/crash_reporter/kernel_collector.cc
index 4fa51e2..652e7ff 100644
--- a/crash_reporter/kernel_collector.cc
+++ b/crash_reporter/kernel_collector.cc
@@ -7,7 +7,6 @@
#include "base/file_util.h"
#include "base/logging.h"
#include "base/string_util.h"
-#include "crash-reporter/system_logging.h"
const char KernelCollector::kClearingSequence[] = " ";
static const char kDefaultKernelStackSignature[] =
@@ -41,8 +40,7 @@
// clear contents since ReadFileToString actually appends to the string.
contents->clear();
if (!file_util::ReadFileToString(preserved_dump_path_, contents)) {
- logger_->LogError("Unable to read %s",
- preserved_dump_path_.value().c_str());
+ LOG(ERROR) << "Unable to read " << preserved_dump_path_.value();
return false;
}
return true;
@@ -50,13 +48,13 @@
bool KernelCollector::Enable() {
if (!file_util::PathExists(preserved_dump_path_)) {
- logger_->LogWarning("Kernel does not support crash dumping");
+ LOG(WARNING) << "Kernel does not support crash dumping";
return false;
}
// To enable crashes, we will eventually need to set
// the chnv bit in BIOS, but it does not yet work.
- logger_->LogInfo("Enabling kernel crash handling");
+ LOG(INFO) << "Enabling kernel crash handling";
is_enabled_ = true;
return true;
}
@@ -68,10 +66,10 @@
preserved_dump_path_,
kClearingSequence,
strlen(kClearingSequence)) != strlen(kClearingSequence)) {
- logger_->LogError("Failed to clear kernel crash dump");
+ LOG(ERROR) << "Failed to clear kernel crash dump";
return false;
}
- logger_->LogInfo("Cleared kernel crash diagnostics");
+ LOG(INFO) << "Cleared kernel crash diagnostics";
return true;
}
@@ -256,10 +254,9 @@
bool feedback = is_feedback_allowed_function_();
- logger_->LogInfo("Received prior crash notification from "
- "kernel (signature %s) (%s)",
- signature.c_str(),
- feedback ? "handling" : "ignoring - no consent");
+ LOG(INFO) << "Received prior crash notification from "
+ << "kernel (signature " << signature << ") ("
+ << (feedback ? "handling" : "ignoring - no consent") << ")";
if (feedback) {
count_crash_function_();
@@ -284,8 +281,8 @@
kernel_dump.data(),
kernel_dump.length()) !=
static_cast<int>(kernel_dump.length())) {
- logger_->LogInfo("Failed to write kernel dump to %s",
- kernel_crash_path.value().c_str());
+ LOG(INFO) << "Failed to write kernel dump to "
+ << kernel_crash_path.value().c_str();
return true;
}
@@ -296,8 +293,7 @@
kKernelExecName,
kernel_crash_path.value());
- logger_->LogInfo("Stored kcrash to %s",
- kernel_crash_path.value().c_str());
+ LOG(INFO) << "Stored kcrash to " << kernel_crash_path.value();
}
if (!ClearPreservedDump()) {
return false;
diff --git a/crash_reporter/kernel_collector_test.cc b/crash_reporter/kernel_collector_test.cc
index 7b84229..f7c88f0 100644
--- a/crash_reporter/kernel_collector_test.cc
+++ b/crash_reporter/kernel_collector_test.cc
@@ -6,8 +6,9 @@
#include "base/file_util.h"
#include "base/string_util.h"
+#include "chromeos/syslog_logging.h"
+#include "chromeos/test_helpers.h"
#include "crash-reporter/kernel_collector.h"
-#include "crash-reporter/system_logging_mock.h"
#include "gflags/gflags.h"
#include "gtest/gtest.h"
@@ -17,6 +18,8 @@
static const char kTestKCrash[] = "test/kcrash";
static const char kTestCrashDirectory[] = "test/crash_directory";
+using chromeos::FindLog;
+
void CountCrash() {
++s_crashes;
}
@@ -30,13 +33,13 @@
s_crashes = 0;
s_metrics = true;
collector_.Initialize(CountCrash,
- IsMetrics,
- &logging_);
+ IsMetrics);
mkdir("test", 0777);
test_kcrash_ = FilePath(kTestKCrash);
collector_.OverridePreservedDumpPath(test_kcrash_);
unlink(kTestKCrash);
mkdir(kTestCrashDirectory, 0777);
+ chromeos::ClearLog();
}
protected:
void WriteStringToFile(const FilePath &file_path,
@@ -48,7 +51,6 @@
void SetUpSuccessfulCollect();
void CheckPreservedDumpClear();
- SystemLoggingMock logging_;
KernelCollector collector_;
FilePath test_kcrash_;
};
@@ -68,10 +70,8 @@
TEST_F(KernelCollectorTest, EnableMissingKernel) {
ASSERT_FALSE(collector_.Enable());
ASSERT_FALSE(collector_.IsEnabled());
- ASSERT_EQ(std::string::npos,
- logging_.log().find("Enabling kernel crash handling"));
- ASSERT_NE(std::string::npos,
- logging_.log().find("Kernel does not support crash dumping"));
+ ASSERT_TRUE(FindLog(
+ "Kernel does not support crash dumping"));
ASSERT_EQ(s_crashes, 0);
}
@@ -79,8 +79,7 @@
WriteStringToFile(test_kcrash_, "");
ASSERT_TRUE(collector_.Enable());
ASSERT_TRUE(collector_.IsEnabled());
- ASSERT_NE(std::string::npos,
- logging_.log().find("Enabling kernel crash handling"));
+ ASSERT_TRUE(FindLog("Enabling kernel crash handling"));
ASSERT_EQ(s_crashes, 0);
}
@@ -97,24 +96,22 @@
TEST_F(KernelCollectorTest, CollectPreservedFileMissing) {
ASSERT_FALSE(collector_.Collect());
- ASSERT_NE(logging_.log().find("Unable to read test/kcrash"),
- std::string::npos);
+ ASSERT_TRUE(FindLog("Unable to read test/kcrash"));
ASSERT_EQ(0, s_crashes);
}
TEST_F(KernelCollectorTest, CollectNoCrash) {
WriteStringToFile(test_kcrash_, "");
ASSERT_FALSE(collector_.Collect());
- ASSERT_EQ(logging_.log().find("Collected kernel crash"),
- std::string::npos);
+ ASSERT_FALSE(FindLog("Collected kernel crash"));
ASSERT_EQ(0, s_crashes);
}
TEST_F(KernelCollectorTest, CollectBadDirectory) {
WriteStringToFile(test_kcrash_, "something");
ASSERT_TRUE(collector_.Collect());
- ASSERT_NE(logging_.log().find(
- "Unable to create appropriate crash directory"), std::string::npos);
+ ASSERT_TRUE(FindLog(
+ "Unable to create appropriate crash directory"));
ASSERT_EQ(1, s_crashes);
}
@@ -135,7 +132,7 @@
SetUpSuccessfulCollect();
s_metrics = false;
ASSERT_TRUE(collector_.Collect());
- ASSERT_NE(std::string::npos, logging_.log().find("(ignoring - no consent)"));
+ ASSERT_TRUE(FindLog("(ignoring - no consent)"));
ASSERT_EQ(0, s_crashes);
CheckPreservedDumpClear();
@@ -145,12 +142,13 @@
SetUpSuccessfulCollect();
ASSERT_TRUE(collector_.Collect());
ASSERT_EQ(1, s_crashes);
- ASSERT_NE(std::string::npos, logging_.log().find("(handling)"));
+ ASSERT_TRUE(FindLog("(handling)"));
static const char kNamePrefix[] = "Stored kcrash to ";
- size_t pos = logging_.log().find(kNamePrefix);
+ std::string log = chromeos::GetLog();
+ size_t pos = log.find(kNamePrefix);
ASSERT_NE(std::string::npos, pos);
pos += strlen(kNamePrefix);
- std::string filename = logging_.log().substr(pos, std::string::npos);
+ std::string filename = log.substr(pos, std::string::npos);
// Take the name up until \n
size_t end_pos = filename.find_first_of("\n");
ASSERT_NE(std::string::npos, end_pos);
@@ -272,6 +270,6 @@
}
int main(int argc, char **argv) {
- ::testing::InitGoogleTest(&argc, argv);
+ SetUpTests(&argc, argv, false);
return RUN_ALL_TESTS();
}
diff --git a/crash_reporter/system_logging.cc b/crash_reporter/system_logging.cc
deleted file mode 100644
index 04bb356..0000000
--- a/crash_reporter/system_logging.cc
+++ /dev/null
@@ -1,55 +0,0 @@
-// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "crash-reporter/system_logging.h"
-
-#include <syslog.h>
-
-#include "base/stringprintf.h"
-
-std::string SystemLoggingImpl::identity_;
-
-SystemLoggingImpl::SystemLoggingImpl() : is_accumulating_(false) {
-}
-
-SystemLoggingImpl::~SystemLoggingImpl() {
-}
-
-void SystemLoggingImpl::Initialize(const char *ident) {
- // Man page does not specify if openlog copies its string or assumes
- // the pointer is always valid, so make its scope global.
- identity_ = ident;
- openlog(identity_.c_str(), LOG_PID, LOG_USER);
-}
-
-void SystemLoggingImpl::LogWithLevel(int level, const char *format,
- va_list arg_list) {
- std::string message = StringPrintV(format, arg_list);
- syslog(level, "%s", message.c_str());
- if (is_accumulating_) {
- accumulator_.append(message);
- accumulator_.push_back('\n');
- }
-}
-
-void SystemLoggingImpl::LogInfo(const char *format, ...) {
- va_list vl;
- va_start(vl, format);
- LogWithLevel(LOG_INFO, format, vl);
- va_end(vl);
-}
-
-void SystemLoggingImpl::LogWarning(const char *format, ...) {
- va_list vl;
- va_start(vl, format);
- LogWithLevel(LOG_WARNING, format, vl);
- va_end(vl);
-}
-
-void SystemLoggingImpl::LogError(const char *format, ...) {
- va_list vl;
- va_start(vl, format);
- LogWithLevel(LOG_ERR, format, vl);
- va_end(vl);
-}
diff --git a/crash_reporter/system_logging.h b/crash_reporter/system_logging.h
deleted file mode 100644
index d1f72a5..0000000
--- a/crash_reporter/system_logging.h
+++ /dev/null
@@ -1,45 +0,0 @@
-// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CRASH_REPORTER_SYSTEM_LOGGING_H_
-#define CRASH_REPORTER_SYSTEM_LOGGING_H_
-
-#include <stdarg.h>
-#include <string>
-
-class SystemLogging {
- public:
- virtual void Initialize(const char *ident) = 0;
- virtual void LogInfo(const char *format, ...) = 0;
- virtual void LogWarning(const char *format, ...) = 0;
- virtual void LogError(const char *format, ...) = 0;
- virtual void set_accumulating(bool value) = 0;
- virtual std::string get_accumulator() = 0;
-};
-
-// SystemLoggingImpl implements SystemLogging but adds the
-// capability of accumulating the log to a STL string.
-class SystemLoggingImpl : public SystemLogging {
- public:
- SystemLoggingImpl();
- virtual ~SystemLoggingImpl();
- virtual void Initialize(const char *ident);
- virtual void LogInfo(const char *format, ...);
- virtual void LogWarning(const char *format, ...);
- virtual void LogError(const char *format, ...);
- virtual void set_accumulating(bool value) {
- is_accumulating_ = value;
- }
- virtual std::string get_accumulator() {
- return accumulator_;
- }
- private:
- static std::string identity_;
- std::string accumulator_;
- bool is_accumulating_;
- void LogWithLevel(int level, const char *format,
- va_list arg_list);
-};
-
-#endif // CRASH_REPORTER_SYSTEM_LOGGING_H_
diff --git a/crash_reporter/system_logging_mock.cc b/crash_reporter/system_logging_mock.cc
deleted file mode 100644
index 04ce853..0000000
--- a/crash_reporter/system_logging_mock.cc
+++ /dev/null
@@ -1,42 +0,0 @@
-// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include <stdarg.h>
-
-#include "base/string_util.h"
-#include "crash-reporter/system_logging_mock.h"
-
-void SystemLoggingMock::LogInfo(const char *format, ...) {
- va_list vl;
- va_start(vl, format);
- log_ += ident_ + "info: ";
- StringAppendV(&log_, format, vl);
- log_ += "\n";
- va_end(vl);
-}
-
-void SystemLoggingMock::LogWarning(const char *format, ...) {
- va_list vl;
- va_start(vl, format);
- log_ += ident_ + "warning: ";
- StringAppendV(&log_, format, vl);
- log_ += "\n";
- va_end(vl);
-}
-
-void SystemLoggingMock::LogError(const char *format, ...) {
- va_list vl;
- va_start(vl, format);
- log_ += ident_ + "error: ";
- StringAppendV(&log_, format, vl);
- log_ += "\n";
- va_end(vl);
-}
-
-void SystemLoggingMock::set_accumulating(bool value) {
-}
-
-std::string SystemLoggingMock::get_accumulator() {
- return "";
-}
diff --git a/crash_reporter/system_logging_mock.h b/crash_reporter/system_logging_mock.h
deleted file mode 100644
index 56c6e58..0000000
--- a/crash_reporter/system_logging_mock.h
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CRASH_REPORTER_SYSTEM_LOGGING_MOCK_H_
-#define CRASH_REPORTER_SYSTEM_LOGGING_MOCK_H_
-
-#include <string>
-
-#include "crash-reporter/system_logging.h"
-
-class SystemLoggingMock : public SystemLogging {
- public:
- void Initialize(const char *ident) {}
- virtual void LogInfo(const char *format, ...);
- virtual void LogWarning(const char *format, ...);
- virtual void LogError(const char *format, ...);
- virtual void set_accumulating(bool value);
- virtual std::string get_accumulator();
-
- const std::string &log() { return log_; }
-
- void clear() { log_.clear(); }
-
- private:
- static std::string identity_;
- std::string log_;
- std::string ident_;
-};
-
-#endif // CRASH_REPORTER_SYSTEM_LOGGING_H_
diff --git a/crash_reporter/test_helpers.h b/crash_reporter/test_helpers.h
deleted file mode 100644
index b515901..0000000
--- a/crash_reporter/test_helpers.h
+++ /dev/null
@@ -1,18 +0,0 @@
-// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef _CRASH_REPORTER_TEST_HELPERS_H_
-#define _CRASH_REPORTER_TEST_HELPERS_H_
-
-#include "gtest/gtest.h"
-
-inline void ExpectFileEquals(const char *golden,
- const char *file_path) {
- std::string contents;
- EXPECT_TRUE(file_util::ReadFileToString(FilePath(file_path),
- &contents));
- EXPECT_EQ(golden, contents);
-}
-
-#endif // _CRASH_REPORTER_TEST_HELPERS_H_
diff --git a/crash_reporter/unclean_shutdown_collector.cc b/crash_reporter/unclean_shutdown_collector.cc
index 1e1c784..ad7386f 100644
--- a/crash_reporter/unclean_shutdown_collector.cc
+++ b/crash_reporter/unclean_shutdown_collector.cc
@@ -6,7 +6,6 @@
#include "base/file_util.h"
#include "base/logging.h"
-#include "crash-reporter/system_logging.h"
static const char kUncleanShutdownFile[] =
"/var/lib/crash_reporter/pending_clean_shutdown";
@@ -32,7 +31,7 @@
FilePath file_path(unclean_shutdown_file_);
file_util::CreateDirectory(file_path.DirName());
if (file_util::WriteFile(file_path, "", 0) != 0) {
- logger_->LogError("Unable to create shutdown check file");
+ LOG(ERROR) << "Unable to create shutdown check file";
return false;
}
return true;
@@ -40,8 +39,8 @@
bool UncleanShutdownCollector::DeleteUncleanShutdownFiles() {
if (!file_util::Delete(FilePath(unclean_shutdown_file_), false)) {
- logger_->LogError("Failed to delete unclean shutdown file %s",
- unclean_shutdown_file_);
+ LOG(ERROR) << "Failed to delete unclean shutdown file "
+ << unclean_shutdown_file_;
return false;
}
// Delete power manager trace files if they exist.
@@ -55,7 +54,7 @@
if (!file_util::PathExists(unclean_file_path)) {
return false;
}
- logger_->LogWarning("Last shutdown was not clean");
+ LOG(WARNING) << "Last shutdown was not clean";
if (DeadBatteryCausedUncleanShutdown()) {
DeleteUncleanShutdownFiles();
return false;
@@ -69,7 +68,7 @@
}
bool UncleanShutdownCollector::Disable() {
- logger_->LogInfo("Clean shutdown signalled");
+ LOG(INFO) << "Clean shutdown signalled";
return DeleteUncleanShutdownFiles();
}
@@ -77,16 +76,16 @@
{
// Check for case of battery running out while suspended.
if (file_util::PathExists(powerd_suspended_file_)) {
- logger_->LogInfo("Unclean shutdown occurred while suspended. Not counting "
- "toward unclean shutdown statistic.");
+ LOG(INFO) << "Unclean shutdown occurred while suspended. Not counting "
+ << "toward unclean shutdown statistic.";
return true;
}
// Check for case of battery running out after resuming from a low-battery
// suspend.
if (file_util::PathExists(powerd_low_battery_file_)) {
- logger_->LogInfo("Unclean shutdown occurred while running with battery "
- "critically low. Not counting toward unclean shutdown "
- "statistic.");
+ LOG(INFO) << "Unclean shutdown occurred while running with battery "
+ << "critically low. Not counting toward unclean shutdown "
+ << "statistic.";
return true;
}
return false;
diff --git a/crash_reporter/unclean_shutdown_collector_test.cc b/crash_reporter/unclean_shutdown_collector_test.cc
index 2cf9fb5..89b0ec9 100644
--- a/crash_reporter/unclean_shutdown_collector_test.cc
+++ b/crash_reporter/unclean_shutdown_collector_test.cc
@@ -6,8 +6,9 @@
#include "base/file_util.h"
#include "base/string_util.h"
+#include "chromeos/syslog_logging.h"
+#include "chromeos/test_helpers.h"
#include "crash-reporter/unclean_shutdown_collector.h"
-#include "crash-reporter/system_logging_mock.h"
#include "gflags/gflags.h"
#include "gtest/gtest.h"
@@ -18,6 +19,8 @@
static const char kTestSuspended[] = "test/suspended";
static const char kTestUnclean[] = "test/unclean";
+using ::chromeos::FindLog;
+
void CountCrash() {
++s_crashes;
}
@@ -30,8 +33,7 @@
void SetUp() {
s_crashes = 0;
collector_.Initialize(CountCrash,
- IsMetrics,
- &logging_);
+ IsMetrics);
rmdir("test");
test_unclean_ = FilePath(kTestUnclean);
collector_.unclean_shutdown_file_ = kTestUnclean;
@@ -39,6 +41,7 @@
// Set up alternate power manager tracing files as well
collector_.powerd_suspended_file_ = FilePath(kTestSuspended);
collector_.powerd_low_battery_file_ = FilePath(kTestLowBattery);
+ chromeos::ClearLog();
}
protected:
void WriteStringToFile(const FilePath &file_path,
@@ -47,7 +50,6 @@
file_util::WriteFile(file_path, data, strlen(data)));
}
- SystemLoggingMock logging_;
UncleanShutdownCollector collector_;
FilePath test_unclean_;
};
@@ -66,8 +68,7 @@
TEST_F(UncleanShutdownCollectorTest, EnableCannotWrite) {
collector_.unclean_shutdown_file_ = "/bad/path";
ASSERT_FALSE(collector_.Enable());
- ASSERT_NE(std::string::npos,
- logging_.log().find("Unable to create shutdown check file"));
+ ASSERT_TRUE(FindLog("Unable to create shutdown check file"));
}
TEST_F(UncleanShutdownCollectorTest, CollectTrue) {
@@ -76,8 +77,7 @@
ASSERT_TRUE(collector_.Collect());
ASSERT_FALSE(file_util::PathExists(test_unclean_));
ASSERT_EQ(1, s_crashes);
- ASSERT_NE(std::string::npos,
- logging_.log().find("Last shutdown was not clean"));
+ ASSERT_TRUE(FindLog("Last shutdown was not clean"));
}
TEST_F(UncleanShutdownCollectorTest, CollectFalse) {
@@ -93,9 +93,8 @@
ASSERT_FALSE(file_util::PathExists(test_unclean_));
ASSERT_FALSE(file_util::PathExists(collector_.powerd_low_battery_file_));
ASSERT_EQ(0, s_crashes);
- ASSERT_NE(std::string::npos,
- logging_.log().find("Unclean shutdown occurred while running with "
- "battery critically low."));
+ ASSERT_TRUE(FindLog(
+ "Unclean shutdown occurred while running with battery critically low."));
}
TEST_F(UncleanShutdownCollectorTest, CollectDeadBatterySuspended) {
@@ -106,8 +105,7 @@
ASSERT_FALSE(file_util::PathExists(test_unclean_));
ASSERT_FALSE(file_util::PathExists(collector_.powerd_suspended_file_));
ASSERT_EQ(0, s_crashes);
- ASSERT_NE(std::string::npos,
- logging_.log().find("Unclean shutdown occurred while suspended."));
+ ASSERT_TRUE(FindLog("Unclean shutdown occurred while suspended."));
}
TEST_F(UncleanShutdownCollectorTest, Disable) {
@@ -130,6 +128,6 @@
}
int main(int argc, char **argv) {
- ::testing::InitGoogleTest(&argc, argv);
+ SetUpTests(&argc, argv, false);
return RUN_ALL_TESTS();
}
diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc
index 6e6c765..bfaaddf 100644
--- a/crash_reporter/user_collector.cc
+++ b/crash_reporter/user_collector.cc
@@ -16,7 +16,8 @@
#include "base/file_util.h"
#include "base/logging.h"
#include "base/string_util.h"
-#include "crash-reporter/system_logging.h"
+#include "chromeos/process.h"
+#include "chromeos/syslog_logging.h"
#include "gflags/gflags.h"
#pragma GCC diagnostic ignored "-Wstrict-aliasing"
@@ -55,11 +56,9 @@
UserCollector::CountCrashFunction count_crash_function,
const std::string &our_path,
UserCollector::IsFeedbackAllowedFunction is_feedback_allowed_function,
- SystemLogging *logger,
bool generate_diagnostics) {
CrashCollector::Initialize(count_crash_function,
- is_feedback_allowed_function,
- logger);
+ is_feedback_allowed_function);
our_path_ = our_path;
initialized_ = true;
generate_diagnostics_ = generate_diagnostics;
@@ -82,13 +81,13 @@
bool UserCollector::SetUpInternal(bool enabled) {
CHECK(initialized_);
- logger_->LogInfo("%s user crash handling",
- enabled ? "Enabling" : "Disabling");
+ LOG(INFO) << (enabled ? "Enabling" : "Disabling") << " user crash handling";
+
if (file_util::WriteFile(FilePath(core_pipe_limit_file_),
kCorePipeLimit,
strlen(kCorePipeLimit)) !=
static_cast<int>(strlen(kCorePipeLimit))) {
- logger_->LogError("Unable to write %s", core_pipe_limit_file_.c_str());
+ LOG(ERROR) << "Unable to write " << core_pipe_limit_file_;
return false;
}
std::string pattern = GetPattern(enabled);
@@ -96,7 +95,7 @@
pattern.c_str(),
pattern.length()) !=
static_cast<int>(pattern.length())) {
- logger_->LogError("Unable to write %s", core_pattern_file_.c_str());
+ LOG(ERROR) << "Unable to write " << core_pattern_file_;
return false;
}
return true;
@@ -115,8 +114,8 @@
ssize_t size = readlink(symlink.value().c_str(), buffer.get(), max_size);
if (size < 0) {
int saved_errno = errno;
- logger_->LogError("Readlink failed on %s with %d",
- symlink.value().c_str(), saved_errno);
+ LOG(ERROR) << "Readlink failed on " << symlink.value() << " with "
+ << saved_errno;
return false;
}
buffer[size] = 0;
@@ -142,19 +141,19 @@
FilePath process_path = GetProcessPath(pid);
FilePath exe_path = process_path.Append("exe");
if (!GetSymlinkTarget(exe_path, &target)) {
- logger_->LogInfo("GetSymlinkTarget failed - Path %s DirectoryExists: %d",
- process_path.value().c_str(),
- file_util::DirectoryExists(process_path));
+ LOG(INFO) << "GetSymlinkTarget failed - Path " << process_path.value()
+ << " DirectoryExists: "
+ << file_util::DirectoryExists(process_path);
// Try to further diagnose exe readlink failure cause.
struct stat buf;
int stat_result = stat(exe_path.value().c_str(), &buf);
int saved_errno = errno;
if (stat_result < 0) {
- logger_->LogInfo("stat %s failed: %d %d", exe_path.value().c_str(),
- stat_result, saved_errno);
+ LOG(INFO) << "stat " << exe_path.value() << " failed: " << stat_result
+ << " " << saved_errno;
} else {
- logger_->LogInfo("stat %s succeeded: st_mode=%d",
- exe_path.value().c_str(), buf.st_mode);
+ LOG(INFO) << "stat " << exe_path.value() << " succeeded: st_mode="
+ << buf.st_mode;
}
return false;
}
@@ -198,13 +197,13 @@
void UserCollector::EnqueueCollectionErrorLog(pid_t pid,
const std::string &exec) {
FilePath crash_path;
- logger_->LogInfo("Writing conversion problems as separate crash report.");
+ LOG(INFO) << "Writing conversion problems as separate crash report.";
if (!GetCreatedCrashDirectoryByEuid(0, &crash_path, NULL)) {
- logger_->LogError("Could not even get log directory; out of space?");
+ LOG(ERROR) << "Could not even get log directory; out of space?";
return;
}
std::string dump_basename = FormatDumpBasename(exec, time(NULL), pid);
- std::string error_log = logger_->get_accumulator();
+ std::string error_log = chromeos::GetLog();
FilePath diag_log_path = GetCrashPath(crash_path, dump_basename, "diaglog");
if (GetLogContents(FilePath(kDefaultLogConfig), kCollectionErrorSignature,
diag_log_path)) {
@@ -229,13 +228,12 @@
bool UserCollector::CopyOffProcFiles(pid_t pid,
const FilePath &container_dir) {
if (!file_util::CreateDirectory(container_dir)) {
- logger_->LogError("Could not create %s",
- container_dir.value().c_str());
+ LOG(ERROR) << "Could not create " << container_dir.value().c_str();
return false;
}
FilePath process_path = GetProcessPath(pid);
if (!file_util::PathExists(process_path)) {
- logger_->LogError("Path %s does not exist", process_path.value().c_str());
+ LOG(ERROR) << "Path " << process_path.value() << " does not exist";
return false;
}
static const char *proc_files[] = {
@@ -248,7 +246,7 @@
for (unsigned i = 0; i < arraysize(proc_files); ++i) {
if (!file_util::CopyFile(process_path.Append(proc_files[i]),
container_dir.Append(proc_files[i]))) {
- logger_->LogError("Could not copy %s file", proc_files[i]);
+ LOG(ERROR) << "Could not copy " << proc_files[i] << " file";
return false;
}
}
@@ -261,26 +259,25 @@
FilePath process_path = GetProcessPath(pid);
std::string status;
if (FLAGS_directory_failure) {
- logger_->LogError("Purposefully failing to create spool directory");
+ LOG(ERROR) << "Purposefully failing to create spool directory";
return false;
}
if (!file_util::ReadFileToString(process_path.Append("status"),
&status)) {
- logger_->LogError("Could not read status file");
- logger_->LogInfo("Path %s DirectoryExists: %d",
- process_path.value().c_str(),
- file_util::DirectoryExists(process_path));
+ LOG(ERROR) << "Could not read status file";
+ LOG(INFO) << "Path " << process_path.value() << " DirectoryExists: "
+ << file_util::DirectoryExists(process_path);
return false;
}
int process_euid;
if (!GetIdFromStatus(kUserId, kIdEffective, status, &process_euid)) {
- logger_->LogError("Could not find euid in status file");
+ LOG(ERROR) << "Could not find euid in status file";
return false;
}
if (!GetCreatedCrashDirectoryByEuid(process_euid,
crash_file_path,
out_of_capacity)) {
- logger_->LogError("Could not create crash directory");
+ LOG(ERROR) << "Could not create crash directory";
return false;
}
return true;
@@ -293,7 +290,7 @@
return true;
}
- logger_->LogError("Could not write core file");
+ LOG(ERROR) << "Could not write core file";
// If the file system was full, make sure we remove any remnants.
file_util::Delete(core_path, false);
return false;
@@ -304,34 +301,32 @@
const FilePath &minidump_path,
const FilePath &temp_directory) {
FilePath output_path = temp_directory.Append("output");
- std::vector<const char *> core2md_arguments;
- core2md_arguments.push_back(kCoreToMinidumpConverterPath);
- core2md_arguments.push_back(core_path.value().c_str());
- core2md_arguments.push_back(procfs_directory.value().c_str());
- core2md_arguments.push_back(minidump_path.value().c_str());
+ chromeos::ProcessImpl core2md;
+ core2md.RedirectOutput(output_path.value());
+ core2md.AddArg(kCoreToMinidumpConverterPath);
+ core2md.AddArg(core_path.value());
+ core2md.AddArg(procfs_directory.value());
- if (FLAGS_core2md_failure) {
+ if (!FLAGS_core2md_failure) {
+ core2md.AddArg(minidump_path.value());
+ } else {
// To test how core2md errors are propagaged, cause an error
// by forgetting a required argument.
- core2md_arguments.pop_back();
}
- int errorlevel = ForkExecAndPipe(core2md_arguments,
- output_path.value().c_str());
+ int errorlevel = core2md.Run();
std::string output;
file_util::ReadFileToString(output_path, &output);
if (errorlevel != 0) {
- logger_->LogError("Problem during %s [result=%d]: %s",
- kCoreToMinidumpConverterPath,
- errorlevel,
- output.c_str());
+ LOG(ERROR) << "Problem during " << kCoreToMinidumpConverterPath
+ << " [result=" << errorlevel << "]: " << output;
return false;
}
if (!file_util::PathExists(minidump_path)) {
- logger_->LogError("Minidump file %s was not created",
- minidump_path.value().c_str());
+ LOG(ERROR) << "Minidump file " << minidump_path.value()
+ << " was not created";
return false;
}
return true;
@@ -356,7 +351,7 @@
container_dir); // temporary directory
if (conversion_result) {
- logger_->LogInfo("Stored minidump to %s", minidump_path.value().c_str());
+ LOG(INFO) << "Stored minidump to " << minidump_path.value();
}
return conversion_result;
@@ -367,7 +362,7 @@
bool *out_of_capacity) {
FilePath crash_path;
if (!GetCreatedCrashDirectory(pid, &crash_path, out_of_capacity)) {
- logger_->LogError("Unable to find/create process-specific crash path");
+ LOG(ERROR) << "Unable to find/create process-specific crash path";
return false;
}
@@ -390,8 +385,8 @@
if (!ConvertCoreToMinidump(pid, container_dir, core_path,
minidump_path)) {
- logger_->LogInfo("Leaving core file at %s due to conversion error",
- core_path.value().c_str());
+ LOG(INFO) << "Leaving core file at " << core_path.value()
+ << " due to conversion error";
return false;
}
@@ -405,8 +400,8 @@
if (!file_util::PathExists(FilePath(kLeaveCoreFile))) {
file_util::Delete(core_path, false);
} else {
- logger_->LogInfo("Leaving core file at %s due to developer image",
- core_path.value().c_str());
+ LOG(INFO) << "Leaving core file at " << core_path.value()
+ << " due to developer image";
}
file_util::Delete(container_dir, true);
@@ -429,7 +424,7 @@
if (!ParseCrashAttributes(crash_attributes, &pid, &signal,
&kernel_supplied_name)) {
- logger_->LogError("Invalid parameter: --user=%s", crash_attributes.c_str());
+ LOG(ERROR) << "Invalid parameter: --user=" << crash_attributes;
return false;
}
@@ -450,8 +445,8 @@
FLAGS_filter_in != exec)) {
// We use a different format message to make it more obvious in tests
// which crashes are test generated and which are real.
- logger_->LogWarning("Ignoring crash from %s[%d] while filter_in=%s.",
- exec.c_str(), pid, FLAGS_filter_in.c_str());
+ LOG(WARNING) << "Ignoring crash from " << exec << "[" << pid << "] while "
+ << "filter_in=" << FLAGS_filter_in << ".";
return true;
}
@@ -469,8 +464,8 @@
handling_string = "ignoring - chrome crash";
}
- logger_->LogWarning("Received crash notification for %s[%d] sig %d (%s)",
- exec.c_str(), pid, signal, handling_string);
+ LOG(WARNING) << "Received crash notification for " << exec << "[" << pid
+ << "] sig " << signal << " (" << handling_string << ")";
if (feedback) {
count_crash_function_();
diff --git a/crash_reporter/user_collector.h b/crash_reporter/user_collector.h
index 21ef14b..e46382c 100644
--- a/crash_reporter/user_collector.h
+++ b/crash_reporter/user_collector.h
@@ -28,7 +28,6 @@
void Initialize(CountCrashFunction count_crash,
const std::string &our_path,
IsFeedbackAllowedFunction is_metrics_allowed,
- SystemLogging *logger,
bool generate_diagnostics);
virtual ~UserCollector();
diff --git a/crash_reporter/user_collector_test.cc b/crash_reporter/user_collector_test.cc
index 9a1d687..93a76ad 100644
--- a/crash_reporter/user_collector_test.cc
+++ b/crash_reporter/user_collector_test.cc
@@ -5,9 +5,9 @@
#include <unistd.h>
#include "base/file_util.h"
-#include "crash-reporter/system_logging_mock.h"
+#include "chromeos/syslog_logging.h"
+#include "chromeos/test_helpers.h"
#include "crash-reporter/user_collector.h"
-#include "crash-reporter/test_helpers.h"
#include "gflags/gflags.h"
#include "gtest/gtest.h"
@@ -16,6 +16,8 @@
static const char kFilePath[] = "/my/path";
+using chromeos::FindLog;
+
void CountCrash() {
++s_crashes;
}
@@ -30,13 +32,13 @@
collector_.Initialize(CountCrash,
kFilePath,
IsMetrics,
- &logging_,
false);
file_util::Delete(FilePath("test"), true);
mkdir("test", 0777);
collector_.set_core_pattern_file("test/core_pattern");
collector_.set_core_pipe_limit_file("test/core_pipe_limit");
pid_ = getpid();
+ chromeos::ClearLog();
}
protected:
void ExpectFileEquals(const char *golden,
@@ -47,7 +49,6 @@
EXPECT_EQ(golden, contents);
}
- SystemLoggingMock logging_;
UserCollector collector_;
pid_t pid_;
};
@@ -57,18 +58,15 @@
ExpectFileEquals("|/my/path --user=%p:%s:%e", "test/core_pattern");
ExpectFileEquals("4", "test/core_pipe_limit");
ASSERT_EQ(s_crashes, 0);
- ASSERT_NE(logging_.log().find("Enabling user crash handling"),
- std::string::npos);
+ EXPECT_TRUE(FindLog("Enabling user crash handling"));
}
TEST_F(UserCollectorTest, EnableNoPatternFileAccess) {
collector_.set_core_pattern_file("/does_not_exist");
ASSERT_FALSE(collector_.Enable());
ASSERT_EQ(s_crashes, 0);
- ASSERT_NE(logging_.log().find("Enabling user crash handling"),
- std::string::npos);
- ASSERT_NE(logging_.log().find("Unable to write /does_not_exist"),
- std::string::npos);
+ EXPECT_TRUE(FindLog("Enabling user crash handling"));
+ EXPECT_TRUE(FindLog("Unable to write /does_not_exist"));
}
TEST_F(UserCollectorTest, EnableNoPipeLimitFileAccess) {
@@ -78,28 +76,23 @@
// Core pattern should not be written if we cannot access the pipe limit
// or otherwise we may set a pattern that results in infinite recursion.
ASSERT_FALSE(file_util::PathExists(FilePath("test/core_pattern")));
- ASSERT_NE(logging_.log().find("Enabling user crash handling"),
- std::string::npos);
- ASSERT_NE(logging_.log().find("Unable to write /does_not_exist"),
- std::string::npos);
+ EXPECT_TRUE(FindLog("Enabling user crash handling"));
+ EXPECT_TRUE(FindLog("Unable to write /does_not_exist"));
}
TEST_F(UserCollectorTest, DisableOK) {
ASSERT_TRUE(collector_.Disable());
ExpectFileEquals("core", "test/core_pattern");
ASSERT_EQ(s_crashes, 0);
- ASSERT_NE(logging_.log().find("Disabling user crash handling"),
- std::string::npos);
+ EXPECT_TRUE(FindLog("Disabling user crash handling"));
}
TEST_F(UserCollectorTest, DisableNoFileAccess) {
collector_.set_core_pattern_file("/does_not_exist");
ASSERT_FALSE(collector_.Disable());
ASSERT_EQ(s_crashes, 0);
- ASSERT_NE(logging_.log().find("Disabling user crash handling"),
- std::string::npos);
- ASSERT_NE(logging_.log().find("Unable to write /does_not_exist"),
- std::string::npos);
+ EXPECT_TRUE(FindLog("Disabling user crash handling"));
+ EXPECT_TRUE(FindLog("Unable to write /does_not_exist"));
}
TEST_F(UserCollectorTest, ParseCrashAttributes) {
@@ -132,40 +125,34 @@
TEST_F(UserCollectorTest, HandleCrashWithoutMetrics) {
s_metrics = false;
collector_.HandleCrash("20:10:ignored", "foobar");
- ASSERT_NE(std::string::npos,
- logging_.log().find(
- "Received crash notification for foobar[20] sig 10"));
+ EXPECT_TRUE(FindLog(
+ "Received crash notification for foobar[20] sig 10"));
ASSERT_EQ(s_crashes, 0);
}
TEST_F(UserCollectorTest, HandleNonChromeCrashWithMetrics) {
s_metrics = true;
collector_.HandleCrash("5:2:ignored", "chromeos-wm");
- ASSERT_NE(std::string::npos,
- logging_.log().find(
- "Received crash notification for chromeos-wm[5] sig 2"));
+ EXPECT_TRUE(FindLog(
+ "Received crash notification for chromeos-wm[5] sig 2"));
ASSERT_EQ(s_crashes, 1);
}
TEST_F(UserCollectorTest, HandleChromeCrashWithMetrics) {
s_metrics = true;
collector_.HandleCrash("5:2:ignored", "chrome");
- ASSERT_NE(std::string::npos,
- logging_.log().find(
- "Received crash notification for chrome[5] sig 2"));
- ASSERT_NE(std::string::npos,
- logging_.log().find("(ignoring - chrome crash)"));
+ EXPECT_TRUE(FindLog(
+ "Received crash notification for chrome[5] sig 2"));
+ EXPECT_TRUE(FindLog("(ignoring - chrome crash)"));
ASSERT_EQ(s_crashes, 0);
}
TEST_F(UserCollectorTest, HandleSuppliedChromeCrashWithMetrics) {
s_metrics = true;
collector_.HandleCrash("0:2:chrome", NULL);
- ASSERT_NE(std::string::npos,
- logging_.log().find(
- "Received crash notification for supplied_chrome[0] sig 2"));
- ASSERT_NE(std::string::npos,
- logging_.log().find("(ignoring - chrome crash)"));
+ EXPECT_TRUE(FindLog(
+ "Received crash notification for supplied_chrome[0] sig 2"));
+ EXPECT_TRUE(FindLog("(ignoring - chrome crash)"));
ASSERT_EQ(s_crashes, 0);
}
@@ -178,9 +165,8 @@
FilePath result;
ASSERT_FALSE(collector_.GetSymlinkTarget(FilePath("/does_not_exist"),
&result));
- ASSERT_NE(std::string::npos,
- logging_.log().find(
- "Readlink failed on /does_not_exist with 2"));
+ ASSERT_TRUE(FindLog(
+ "Readlink failed on /does_not_exist with 2"));
std::string long_link;
for (int i = 0; i < 50; ++i)
long_link += "0123456789";
@@ -201,23 +187,16 @@
TEST_F(UserCollectorTest, GetExecutableBaseNameFromPid) {
std::string base_name;
EXPECT_FALSE(collector_.GetExecutableBaseNameFromPid(0, &base_name));
- EXPECT_NE(std::string::npos,
- logging_.log().find(
- "Readlink failed on /proc/0/exe with 2"));
- EXPECT_NE(std::string::npos,
- logging_.log().find(
- "GetSymlinkTarget failed - Path "
- "/proc/0 DirectoryExists: 0"));
- EXPECT_NE(std::string::npos,
- logging_.log().find(
- "stat /proc/0/exe failed: -1 2"));
+ EXPECT_TRUE(FindLog(
+ "Readlink failed on /proc/0/exe with 2"));
+ EXPECT_TRUE(FindLog(
+ "GetSymlinkTarget failed - Path /proc/0 DirectoryExists: 0"));
+ EXPECT_TRUE(FindLog("stat /proc/0/exe failed: -1 2"));
- logging_.clear();
+ chromeos::ClearLog();
pid_t my_pid = getpid();
EXPECT_TRUE(collector_.GetExecutableBaseNameFromPid(my_pid, &base_name));
- EXPECT_EQ(std::string::npos,
- logging_.log().find(
- "Readlink failed"));
+ EXPECT_FALSE(FindLog("Readlink failed"));
EXPECT_EQ("user_collector_test", base_name);
}
@@ -303,24 +282,19 @@
TEST_F(UserCollectorTest, CopyOffProcFilesBadPath) {
// Try a path that is not writable.
ASSERT_FALSE(collector_.CopyOffProcFiles(pid_, FilePath("/bad/path")));
- ASSERT_NE(logging_.log().find(
- "Could not create /bad/path"),
- std::string::npos);
+ EXPECT_TRUE(FindLog("Could not create /bad/path"));
}
TEST_F(UserCollectorTest, CopyOffProcFilesBadPid) {
FilePath container_path("test/container");
ASSERT_FALSE(collector_.CopyOffProcFiles(0, container_path));
- ASSERT_NE(logging_.log().find(
- "Path /proc/0 does not exist"),
- std::string::npos);
+ EXPECT_TRUE(FindLog("Path /proc/0 does not exist"));
}
TEST_F(UserCollectorTest, CopyOffProcFilesOK) {
FilePath container_path("test/container");
ASSERT_TRUE(collector_.CopyOffProcFiles(pid_, container_path));
- ASSERT_EQ(logging_.log().find(
- "Could not copy"), std::string::npos);
+ EXPECT_FALSE(FindLog("Could not copy"));
static struct {
const char *name;
bool exists;
@@ -342,6 +316,6 @@
}
int main(int argc, char **argv) {
- ::testing::InitGoogleTest(&argc, argv);
+ SetUpTests(&argc, argv, false);
return RUN_ALL_TESTS();
}