diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc index f2f514694..e91f70a12 100644 --- a/crash_reporter/crash_collector.cc +++ b/crash_reporter/crash_collector.cc @@ -8,12 +8,15 @@ #include // For struct passwd. #include // for mode_t. +#include + #include "base/file_util.h" #include "base/logging.h" #include "base/string_util.h" #include "crash-reporter/system_logging.h" static const char kDefaultUserName[] = "chronos"; +static const char kLsbRelease[] = "/etc/lsb-release"; static const char kSystemCrashPath[] = "/var/spool/crash"; static const char kUserCrashPath[] = "/home/chronos/user/crash"; @@ -55,13 +58,23 @@ void CrashCollector::Initialize( logger_ = logger; } +std::string CrashCollector::Sanitize(const std::string &name) { + std::string result = name; + for (size_t i = 0; i < name.size(); ++i) { + if (!isalnum(result[i]) && result[i] != '_') + result[i] = '_'; + } + return result; +} + std::string CrashCollector::FormatDumpBasename(const std::string &exec_name, time_t timestamp, pid_t pid) { struct tm tm; localtime_r(×tamp, &tm); + std::string sanitized_exec_name = Sanitize(exec_name); return StringPrintf("%s.%04d%02d%02d.%02d%02d%02d.%d", - exec_name.c_str(), + sanitized_exec_name.c_str(), tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, @@ -173,22 +186,27 @@ bool CrashCollector::CheckHasCapacity(const FilePath &crash_directory) { } struct dirent ent_buf; struct dirent* ent; - int count_non_core = 0; - int count_core = 0; bool full = false; + std::set basenames; while (readdir_r(dir, &ent_buf, &ent) == 0 && ent != NULL) { if ((strcmp(ent->d_name, ".") == 0) || (strcmp(ent->d_name, "..") == 0)) continue; - if (strcmp(ent->d_name + strlen(ent->d_name) - 5, ".core") == 0) { - ++count_core; - } else { - ++count_non_core; - } + std::string filename(ent->d_name); + size_t last_dot = filename.rfind("."); + std::string basename; + // If there is a valid looking extension, use the base part of the + // name. If the only dot is the first byte (aka a dot file), treat + // it as unique to avoid allowing a directory full of dot files + // from accumulating. + if (last_dot != std::string::npos && last_dot != 0) + basename = filename.substr(0, last_dot); + else + basename = filename; + basenames.insert(basename); - if (count_core >= kMaxCrashDirectorySize || - count_non_core >= kMaxCrashDirectorySize) { + if (basenames.size() >= static_cast(kMaxCrashDirectorySize)) { logger_->LogWarning( "Crash directory %s already full with %d pending reports", crash_directory.value().c_str(), @@ -200,3 +218,53 @@ bool CrashCollector::CheckHasCapacity(const FilePath &crash_directory) { closedir(dir); return !full; } + +bool CrashCollector::ReadKeyValueFile( + const FilePath &path, + const char separator, + std::map *dictionary) { + std::string contents; + if (!file_util::ReadFileToString(path, &contents)) { + return false; + } + typedef std::vector StringVector; + StringVector lines; + SplitString(contents, '\n', &lines); + bool any_errors = false; + for (StringVector::iterator line = lines.begin(); line != lines.end(); + ++line) { + // Allow empty strings. + if (line->empty()) + continue; + StringVector sides; + SplitString(*line, separator, &sides); + if (sides.size() != 2) { + any_errors = true; + continue; + } + dictionary->insert(std::pair(sides[0], sides[1])); + } + return !any_errors; +} + +void CrashCollector::WriteCrashMetaData(const FilePath &meta_path, + const std::string &exec_name) { + std::map contents; + if (!ReadKeyValueFile(FilePath(std::string(kLsbRelease)), '=', &contents)) { + logger_->LogError("Problem parsing %s", kLsbRelease); + // Even though there was some failure, take as much as we could read. + } + std::string version("unknown"); + std::map::iterator i; + if ((i = contents.find("CHROMEOS_RELEASE_VERSION")) != contents.end()) { + version = i->second; + } + std::string meta_data = StringPrintf("exec_name=%s\n" + "ver=%s\n" + "done=1\n", + exec_name.c_str(), + version.c_str()); + if (!file_util::WriteFile(meta_path, meta_data.c_str(), meta_data.size())) { + logger_->LogError("Unable to write %s", meta_path.value().c_str()); + } +} diff --git a/crash_reporter/crash_collector.h b/crash_reporter/crash_collector.h index a75d79148..730a8e761 100644 --- a/crash_reporter/crash_collector.h +++ b/crash_reporter/crash_collector.h @@ -5,9 +5,11 @@ #ifndef _CRASH_REPORTER_CRASH_COLLECTOR_H_ #define _CRASH_REPORTER_CRASH_COLLECTOR_H_ -#include #include +#include +#include + #include "gtest/gtest_prod.h" // for FRIEND_TEST class FilePath; @@ -32,15 +34,22 @@ class CrashCollector { protected: friend class CrashCollectorTest; - FRIEND_TEST(CrashCollectorTest, CheckHasCapacityOverCore); - FRIEND_TEST(CrashCollectorTest, CheckHasCapacityOverNonCore); + FRIEND_TEST(CrashCollectorTest, CheckHasCapacityCorrectBasename); + FRIEND_TEST(CrashCollectorTest, CheckHasCapacityStrangeNames); + FRIEND_TEST(CrashCollectorTest, CheckHasCapacityUsual); FRIEND_TEST(CrashCollectorTest, GetCrashDirectoryInfo); FRIEND_TEST(CrashCollectorTest, FormatDumpBasename); FRIEND_TEST(CrashCollectorTest, Initialize); + FRIEND_TEST(CrashCollectorTest, ReadKeyValueFile); + FRIEND_TEST(CrashCollectorTest, Sanitize); // Set maximum enqueued crashes in a crash directory. static const int kMaxCrashDirectorySize; + // Return a filename that has only [a-z0-1_] characters by mapping + // all others into '_'. + std::string Sanitize(const std::string &name); + // For testing, set the directory always returned by // GetCreatedCrashDirectoryByEuid. void ForceCrashDirectory(const char *forced_directory) { @@ -72,6 +81,16 @@ class CrashCollector { // crash. bool CheckHasCapacity(const FilePath &crash_directory); + // Read the given file of form [\n...] and return + // a map of its contents. + bool ReadKeyValueFile(const FilePath &file, + char separator, + std::map *dictionary); + + // Write a file of metadata about crash. + void WriteCrashMetaData(const FilePath &meta_path, + const std::string &exec_name); + CountCrashFunction count_crash_function_; IsFeedbackAllowedFunction is_feedback_allowed_function_; SystemLogging *logger_; diff --git a/crash_reporter/crash_collector_test.cc b/crash_reporter/crash_collector_test.cc index ed8fff756..2dbf2bb2d 100644 --- a/crash_reporter/crash_collector_test.cc +++ b/crash_reporter/crash_collector_test.cc @@ -48,6 +48,16 @@ TEST_F(CrashCollectorTest, Initialize) { ASSERT_TRUE(&logging_ == collector_.logger_); } +TEST_F(CrashCollectorTest, Sanitize) { + EXPECT_EQ("chrome", collector_.Sanitize("chrome")); + EXPECT_EQ("CHROME", collector_.Sanitize("CHROME")); + EXPECT_EQ("1chrome2", collector_.Sanitize("1chrome2")); + EXPECT_EQ("chrome__deleted_", collector_.Sanitize("chrome (deleted)")); + EXPECT_EQ("foo_bar", collector_.Sanitize("foo.bar")); + EXPECT_EQ("", collector_.Sanitize("")); + EXPECT_EQ("_", collector_.Sanitize(" ")); +} + TEST_F(CrashCollectorTest, GetCrashDirectoryInfo) { FilePath path; const int kRootUid = 0; @@ -118,41 +128,96 @@ bool CrashCollectorTest::CheckHasCapacity() { return has_capacity; } -TEST_F(CrashCollectorTest, CheckHasCapacityOverNonCore) { - // Test up to kMaxCrashDirectorySize-1 non-core files can be added. +TEST_F(CrashCollectorTest, CheckHasCapacityUsual) { + // Test kMaxCrashDirectorySize - 1 non-meta files can be added. for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize - 1; ++i) { - EXPECT_TRUE(CheckHasCapacity()); - file_util::WriteFile(test_dir_.Append(StringPrintf("file%d", i)), "", 0); - } - - // Test an additional kMaxCrashDirectorySize - 1 core files fit. - for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize - 1; ++i) { - EXPECT_TRUE(CheckHasCapacity()); file_util::WriteFile(test_dir_.Append(StringPrintf("file%d.core", i)), "", 0); + EXPECT_TRUE(CheckHasCapacity()); } - // Test an additional kMaxCrashDirectorySize non-core files don't fit. + // Test an additional kMaxCrashDirectorySize - 1 meta files fit. + for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize - 1; ++i) { + file_util::WriteFile(test_dir_.Append(StringPrintf("file%d.meta", i)), + "", 0); + EXPECT_TRUE(CheckHasCapacity()); + } + + // Test an additional kMaxCrashDirectorySize meta files don't fit. for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize; ++i) { - file_util::WriteFile(test_dir_.Append(StringPrintf("overage%d", i)), "", 0); + file_util::WriteFile(test_dir_.Append(StringPrintf("overage%d.meta", i)), + "", 0); EXPECT_FALSE(CheckHasCapacity()); } } -TEST_F(CrashCollectorTest, CheckHasCapacityOverCore) { - // Set up kMaxCrashDirectorySize - 1 core files. +TEST_F(CrashCollectorTest, CheckHasCapacityCorrectBasename) { + // Test kMaxCrashDirectorySize - 1 files can be added. for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize - 1; ++i) { - file_util::WriteFile(test_dir_.Append(StringPrintf("file%d.core", i)), + file_util::WriteFile(test_dir_.Append(StringPrintf("file.%d.core", i)), "", 0); + EXPECT_TRUE(CheckHasCapacity()); } - - EXPECT_TRUE(CheckHasCapacity()); - - // Test an additional core file does not fit. - file_util::WriteFile(test_dir_.Append("overage.core"), "", 0); + file_util::WriteFile(test_dir_.Append("file.last.core"), "", 0); EXPECT_FALSE(CheckHasCapacity()); } +TEST_F(CrashCollectorTest, CheckHasCapacityStrangeNames) { + // Test many files with different extensions and same base fit. + for (int i = 0; i < 5 * CrashCollector::kMaxCrashDirectorySize; ++i) { + file_util::WriteFile(test_dir_.Append(StringPrintf("a.%d", i)), "", 0); + EXPECT_TRUE(CheckHasCapacity()); + } + // Test dot files are treated as individual files. + for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize - 2; ++i) { + file_util::WriteFile(test_dir_.Append(StringPrintf(".file%d", i)), "", 0); + EXPECT_TRUE(CheckHasCapacity()); + } + file_util::WriteFile(test_dir_.Append("normal.meta"), "", 0); + EXPECT_FALSE(CheckHasCapacity()); +} + +TEST_F(CrashCollectorTest, ReadKeyValueFile) { + const char *contents = ("a=b\n" + "\n" + " c=d \n"); + FilePath path(test_dir_.Append("keyval")); + std::map dictionary; + std::map::iterator i; + + file_util::WriteFile(path, contents, strlen(contents)); + + EXPECT_TRUE(collector_.ReadKeyValueFile(path, '=', &dictionary)); + i = dictionary.find("a"); + EXPECT_TRUE(i != dictionary.end() && i->second == "b"); + i = dictionary.find("c"); + EXPECT_TRUE(i != dictionary.end() && i->second == "d"); + + dictionary.clear(); + + contents = ("a=b c d\n" + "e\n" + " f g = h\n" + "i=j\n" + "=k\n" + "l=\n"); + file_util::WriteFile(path, contents, strlen(contents)); + + EXPECT_FALSE(collector_.ReadKeyValueFile(path, '=', &dictionary)); + i = dictionary.find("a"); + EXPECT_TRUE(i != dictionary.end() && i->second == "b c d"); + i = dictionary.find("e"); + EXPECT_TRUE(i == dictionary.end()); + i = dictionary.find("f g"); + EXPECT_TRUE(i != dictionary.end() && i->second == "h"); + i = dictionary.find("i"); + EXPECT_TRUE(i != dictionary.end() && i->second == "j"); + i = dictionary.find(""); + EXPECT_TRUE(i != dictionary.end() && i->second == "k"); + i = dictionary.find("l"); + EXPECT_TRUE(i != dictionary.end() && i->second == ""); +} + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/crash_reporter/crash_reporter.cc b/crash_reporter/crash_reporter.cc index b57bfc40b..d85838698 100644 --- a/crash_reporter/crash_reporter.cc +++ b/crash_reporter/crash_reporter.cc @@ -43,9 +43,7 @@ static MetricsLibrary s_metrics_lib; static SystemLoggingImpl s_system_log; static bool IsFeedbackAllowed() { - // Once crosbug.com/5814 is fixed, call the is opted in function - // here. - return true; + return s_metrics_lib.AreMetricsEnabled(); } static bool TouchFile(const FilePath &file_path) { diff --git a/crash_reporter/crash_sender b/crash_reporter/crash_sender index c62f88aa4..f6b3e3a58 100644 --- a/crash_reporter/crash_sender +++ b/crash_reporter/crash_sender @@ -21,8 +21,15 @@ CONSENT_ID="/home/chronos/Consent To Send Stats" # Path to find which is required for computing the crash rate. FIND="/usr/bin/find" +# Set this to 1 in the environment to allow uploading crash reports +# for unofficial versions. +FORCE_OFFICIAL=${FORCE_OFFICIAL:-0} + +# Path to hardware class description. +HWCLASS_PATH="/sys/devices/platform/chromeos_acpi/HWID" + # Maximum crashes to send per day. -MAX_CRASH_RATE=32 +MAX_CRASH_RATE=${MAX_CRASH_RATE:-32} # File whose existence mocks crash sending. If empty we pretend the # crash sending was successful, otherwise unsuccessful. @@ -32,9 +39,6 @@ MOCK_CRASH_SENDING="/tmp/mock-crash-sending" # Must be stateful to enable testing kernel crashes. PAUSE_CRASH_SENDING="/var/lib/crash_sender_paused" -# URL to send non-official build crash reports to. -REPORT_UPLOAD_STAGING_URL="http://clients2.google.com/cr/staging_report" - # URL to send official build crash reports to. REPORT_UPLOAD_PROD_URL="http://clients2.google.com/cr/report" @@ -89,11 +93,8 @@ check_not_already_running() { exit 1 } -get_version() { - grep ^CHROMEOS_RELEASE_VERSION /etc/lsb-release | cut -d = -f 2- -} - is_official() { + [ ${FORCE_OFFICIAL} -ne 0 ] && return 0 grep ^CHROMEOS_RELEASE_DESCRIPTION /etc/lsb-release | grep -q Official } @@ -119,7 +120,8 @@ is_on_3g() { check_rate() { mkdir -p ${TIMESTAMPS_DIR} # Only consider minidumps written in the past 24 hours by removing all older. - ${FIND} "${TIMESTAMPS_DIR}" -mindepth 1 -mmin +$((24 * 60)) -exec rm '{}' ';' + ${FIND} "${TIMESTAMPS_DIR}" -mindepth 1 -mmin +$((24 * 60)) \ + -exec rm -- '{}' ';' local sends_in_24hrs=$(echo "${TIMESTAMPS_DIR}"/* | wc -w) lecho "Current send rate: ${sends_in_24hrs}sends/24hrs" if [ ${sends_in_24hrs} -ge ${MAX_CRASH_RATE} ]; then @@ -132,37 +134,81 @@ check_rate() { return 0 } -# Return if $1 is a .core file -get_kind() { - local kind="${1##*.}" - if [ "${kind}" = "dmp" ]; then - kind="minidump" - fi - echo ${kind} +# Gets the base part of a crash report file, such as +# name.01234.5678.9012 from name.01234.5678.9012.meta +get_base() { + echo "${1%.*}" } -get_exec_name() { - local filename=$(basename "$1") - echo "${filename%%.*}" +# Return which kind of report the given metadata file relates to +get_kind() { + # There should never be a report with both a dmp and kcrash file. + # If that were to happen we arbitrarily consider this a minidump + # report and effectively ignore the kcrash. + local base="$(get_base "$1")" + if [ -r "${base}.dmp" ]; then + echo "minidump" + return + fi + if [ -r "${base}.kcrash" ]; then + echo "kcrash" + return + fi +} + +get_key_value() { + if ! grep -q "$2=" "$1"; then + echo "undefined" + return + fi + grep "$2=" "$1" | cut -d = -f 2- +} + +# Returns true if mock is enabled. +is_mock() { + [ -f "${MOCK_CRASH_SENDING}" ] && return 0 + return 1 +} + +# Return the board name. +get_board() { + echo $(get_key_value "/etc/lsb-release" "CHROMEOS_RELEASE_BOARD") +} + +# Return the hardware class or "unknown". +get_hardware_class() { + if [ -r "${HWCLASS_PATH}" ]; then + cat "${HWCLASS_PATH}" + else + echo "unknown" + fi } send_crash() { - local report_path="$1" - local kind=$(get_kind "${report_path}") - local exec_name=$(get_exec_name "${report_path}") + local meta_path="$1" + local kind="$(get_kind "${meta_path}")" + local exec_name="$(get_key_value "${meta_path}" "exec_name")" local sleep_time=$(generate_uniform_random $SECONDS_SEND_SPREAD) - local url="${REPORT_UPLOAD_STAGING_URL}" - if is_official; then - url="${REPORT_UPLOAD_PROD_URL}" - fi + local url="${REPORT_UPLOAD_PROD_URL}" + local chromeos_version="$(get_key_value "${meta_path}" "ver")" + local board="$(get_board)" + local hwclass="$(get_hardware_class)" + local payload_extension="${kind}" + [ "${kind}" = "minidump" ] && payload_extension="dmp" + local report_payload="$(get_base "${meta_path}").${payload_extension}" lecho "Sending crash:" lecho " Scheduled to send in ${sleep_time}s" - lecho " Report: ${report_path} (${kind})" - lecho " URL: ${url}" - lecho " Product: ${CHROMEOS_PRODUCT}" + lecho " Metadata: ${meta_path} (${kind})" + lecho " Payload: ${report_payload}" lecho " Version: ${chromeos_version}" + if is_mock; then + lecho " Product: ${CHROMEOS_PRODUCT}" + lecho " URL: ${url}" + lecho " Board: ${board}" + lecho " HWClass: ${hwclass}" + fi lecho " Exec name: ${exec_name}" - if [ -f "${MOCK_CRASH_SENDING}" ]; then + if is_mock; then local mock_in=$(cat "${MOCK_CRASH_SENDING}") if [ "${mock_in}" = "" ]; then lecho "Mocking successful send" @@ -185,7 +231,9 @@ send_crash() { curl "${url}" \ -F "prod=${CHROMEOS_PRODUCT}" \ -F "ver=${chromeos_version}" \ - -F "upload_file_${kind}=@${report_path}" \ + -F "upload_file_${kind}=@${report_payload}" \ + -F "board=${board}" \ + -F "hwclass=${hwclass}" \ -F "exec_name=${exec_name}" \ -F "guid=<${CONSENT_ID}" -o "${report_id}" 2>"${curl_stderr}" curl_result=$? @@ -202,53 +250,98 @@ send_crash() { return ${curl_result} } +# *.meta files always end with done=1 so we can tell if they are complete. +is_complete_metadata() { + grep -q "done=1" "$1" +} + +# Remove the given report path. +remove_report() { + local base="${1%.*}" + rm -f -- "${base}".* +} + # Send all crashes from the given directory. send_crashes() { local dir="$1" - lecho "Considering crashes in ${dir}" # Cycle through minidumps, most recent first. That way if we're about # to exceed the daily rate, we send the most recent minidumps. if [ ! -d "${dir}" ]; then return fi - for file in $(ls -1t "${dir}"); do - local report_path="${dir}/${file}" - lecho "Considering file ${report_path}" - local kind=$(get_kind "${report_path}") - if [ "${kind}" = "core" ]; then - lecho "Ignoring core file." - continue - elif [ "${kind}" != "minidump" ] && [ "${kind}" != "kcrash" ]; then - lecho "Unknown report kind: ${kind}. Removing report." - rm -f "${report_path}" + # Consider any old files which still have no corresponding meta file + # as orphaned, and remove them. + for old_file in $(${FIND} "${dir}" -mindepth 1 \ + -mmin +$((24 * 60)) -type f); do + if [ ! -e "$(get_base "${old_file}").meta" ]; then + lecho "Removing old orphaned file: ${old_file}." + rm -f -- "${old_file}" + fi + done + + # Look through all metadata (*.meta) files, if any exist. + for meta_path in $(ls -1t "${dir}"/*.meta 2>/dev/null); do + lecho "Considering metadata ${meta_path}." + local kind=$(get_kind "${meta_path}") + + if [ "${kind}" != "minidump" ] && [ "${kind}" != "kcrash" ]; then + lecho "Unknown report kind. Removing report." + remove_report "${meta_path}" continue fi - if ! check_rate; then - lecho "Sending ${report_path} would exceed rate. Leaving for later." - return 0 - fi - local chromeos_version=$(get_version) + if is_feedback_disabled; then lecho "Uploading is disabled. Removing crash." - rm "${report_path}" - elif is_on_3g; then - lecho "Not sending crash report while on 3G, saving for later." - elif send_crash "${report_path}"; then - # Send was successful, now remove - lecho "Successfully sent crash ${report_path} and removing" - rm "${report_path}" - else - lecho "Problem sending ${report_path}, not removing" + remove_report "${meta_path}" + continue fi + + if ! is_mock && ! is_official; then + lecho "Not an official OS version. Removing crash." + remove_report "${meta_path}" + continue + fi + + if is_on_3g; then + lecho "Not sending crash reports while on 3G, saving for later." + return 0 + fi + + if ! is_complete_metadata "${meta_path}"; then + # This report is incomplete, so if it's old, just remove it. + local old_meta=$(${FIND} "${dir}" -mindepth 1 -name \ + $(basename "${meta_path}") -mmin +$((24 * 60)) -type f) + if [ -n "${old_meta}" ]; then + lecho "Removing old incomplete metadata." + remove_report "${meta_path}" + else + lecho "Ignoring recent incomplete metadata." + fi + continue + fi + + if ! check_rate; then + lecho "Sending ${meta_path} would exceed rate. Leaving for later." + return 0 + fi + + if ! send_crash "${meta_path}"; then + lecho "Problem sending ${meta_path}, not removing." + continue + fi + + # Send was successful, now remove. + lecho "Successfully sent crash ${meta_path} and removing." + remove_report "${meta_path}" done } main() { trap cleanup EXIT INT TERM if [ -e "${PAUSE_CRASH_SENDING}" ]; then - lecho "Exiting early due to ${PAUSE_CRASH_SENDING}" + lecho "Exiting early due to ${PAUSE_CRASH_SENDING}." exit 1 fi diff --git a/crash_reporter/kernel_collector.cc b/crash_reporter/kernel_collector.cc index 1a79c9d86..f5e20b901 100644 --- a/crash_reporter/kernel_collector.cc +++ b/crash_reporter/kernel_collector.cc @@ -66,17 +66,6 @@ bool KernelCollector::ClearPreservedDump() { return true; } -FilePath KernelCollector::GetKernelCrashPath( - const FilePath &root_crash_path, - time_t timestamp) { - std::string dump_basename = - FormatDumpBasename(kKernelExecName, - timestamp, - kKernelPid); - return root_crash_path.Append( - StringPrintf("%s.kcrash", dump_basename.c_str())); -} - bool KernelCollector::Collect() { std::string kernel_dump; FilePath root_crash_directory; @@ -86,6 +75,8 @@ bool KernelCollector::Collect() { if (kernel_dump.empty()) { return false; } + logger_->LogInfo("Received prior crash notification from kernel"); + if (is_feedback_allowed_function_()) { count_crash_function_(); @@ -94,8 +85,13 @@ bool KernelCollector::Collect() { return true; } - FilePath kernel_crash_path = GetKernelCrashPath(root_crash_directory, - time(NULL)); + std::string dump_basename = + FormatDumpBasename(kKernelExecName, + time(NULL), + kKernelPid); + FilePath kernel_crash_path = root_crash_directory.Append( + StringPrintf("%s.kcrash", dump_basename.c_str())); + if (file_util::WriteFile(kernel_crash_path, kernel_dump.data(), kernel_dump.length()) != @@ -105,6 +101,11 @@ bool KernelCollector::Collect() { return true; } + WriteCrashMetaData( + root_crash_directory.Append( + StringPrintf("%s.meta", dump_basename.c_str())), + kKernelExecName); + logger_->LogInfo("Collected kernel crash diagnostics into %s", kernel_crash_path.value().c_str()); } else { diff --git a/crash_reporter/kernel_collector.h b/crash_reporter/kernel_collector.h index 3e2944245..e6cd57392 100644 --- a/crash_reporter/kernel_collector.h +++ b/crash_reporter/kernel_collector.h @@ -37,14 +37,11 @@ class KernelCollector : public CrashCollector { private: friend class KernelCollectorTest; FRIEND_TEST(KernelCollectorTest, ClearPreservedDump); - FRIEND_TEST(KernelCollectorTest, GetKernelCrashPath); FRIEND_TEST(KernelCollectorTest, LoadPreservedDump); FRIEND_TEST(KernelCollectorTest, CollectOK); bool LoadPreservedDump(std::string *contents); bool ClearPreservedDump(); - FilePath GetKernelCrashPath(const FilePath &root_crash_path, - time_t timestamp); bool is_enabled_; FilePath preserved_dump_path_; diff --git a/crash_reporter/kernel_collector_test.cc b/crash_reporter/kernel_collector_test.cc index 7188e2f15..44a5ba0a7 100644 --- a/crash_reporter/kernel_collector_test.cc +++ b/crash_reporter/kernel_collector_test.cc @@ -95,21 +95,6 @@ TEST_F(KernelCollectorTest, ClearPreservedDump) { ASSERT_EQ(KernelCollector::kClearingSequence, dump); } -TEST_F(KernelCollectorTest, GetKernelCrashPath) { - FilePath root("/var/spool/crash"); - struct tm tm = {0}; - tm.tm_sec = 15; - tm.tm_min = 50; - tm.tm_hour = 13; - tm.tm_mday = 23; - tm.tm_mon = 4; - tm.tm_year = 110; - tm.tm_isdst = -1; - FilePath path = collector_.GetKernelCrashPath(root, mktime(&tm)); - ASSERT_EQ("/var/spool/crash/kernel.20100523.135015.0.kcrash", - path.value()); -} - TEST_F(KernelCollectorTest, CollectPreservedFileMissing) { ASSERT_FALSE(collector_.Collect()); ASSERT_NE(logging_.log().find("Unable to read test/kcrash"), diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc index e051a4ac3..18ab7b95d 100644 --- a/crash_reporter/user_collector.cc +++ b/crash_reporter/user_collector.cc @@ -275,6 +275,11 @@ bool UserCollector::GenerateDiagnostics(pid_t pid, conversion_result = false; } + WriteCrashMetaData( + crash_path.Append( + StringPrintf("%s.meta", dump_basename.c_str())), + exec_name); + if (conversion_result) { logger_->LogInfo("Stored minidump to %s", minidump_path.value().c_str()); } @@ -298,10 +303,12 @@ bool UserCollector::HandleCrash(int signal, int pid, const char *force_exec) { // failing by indicating an unknown name. exec = "unknown"; } - logger_->LogWarning("Received crash notification for %s[%d] sig %d", - exec.c_str(), pid, signal); + bool feedback = is_feedback_allowed_function_(); + logger_->LogWarning("Received crash notification for %s[%d] sig %d (%s)", + exec.c_str(), pid, signal, + feedback ? "handling" : "ignoring"); - if (is_feedback_allowed_function_()) { + if (feedback) { count_crash_function_(); if (generate_diagnostics_) {