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/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_();