crash-reporter: Fix C++ style issues in KernelCollector.
BUG=None
TEST=`FEATURES=test emerge-$BOARD crash-reporter`
Change-Id: Ib33a23059878380425f6eb79385dc67141ad0f77
Reviewed-on: https://chromium-review.googlesource.com/209746
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
diff --git a/crash_reporter/crash_reporter.cc b/crash_reporter/crash_reporter.cc
index 8aafc46..2a74f13 100644
--- a/crash_reporter/crash_reporter.cc
+++ b/crash_reporter/crash_reporter.cc
@@ -130,7 +130,7 @@
bool was_kernel_crash = false;
bool was_unclean_shutdown = false;
kernel_collector->Enable();
- if (kernel_collector->IsEnabled()) {
+ if (kernel_collector->is_enabled()) {
was_kernel_crash = kernel_collector->Collect();
}
diff --git a/crash_reporter/kernel_collector.cc b/crash_reporter/kernel_collector.cc
index 8606369..08ad85b 100644
--- a/crash_reporter/kernel_collector.cc
+++ b/crash_reporter/kernel_collector.cc
@@ -11,23 +11,27 @@
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
-static const char kDefaultKernelStackSignature[] =
- "kernel-UnspecifiedStackSignature";
-static const char kDumpPath[] = "/dev/pstore";
-static const char kDumpFormat[] = "dmesg-ramoops-%zu";
-static const char kKernelExecName[] = "kernel";
+using base::FilePath;
+using base::StringPrintf;
+
+namespace {
+
+const char kDefaultKernelStackSignature[] = "kernel-UnspecifiedStackSignature";
+const char kDumpPath[] = "/dev/pstore";
+const char kDumpFormat[] = "dmesg-ramoops-%zu";
+const char kKernelExecName[] = "kernel";
// Maximum number of records to examine in the kDumpPath.
-static const size_t kMaxDumpRecords = 100;
+const size_t kMaxDumpRecords = 100;
const pid_t kKernelPid = 0;
-static const char kKernelSignatureKey[] = "sig";
+const char kKernelSignatureKey[] = "sig";
// Byte length of maximum human readable portion of a kernel crash signature.
-static const int kMaxHumanStringLength = 40;
+const int kMaxHumanStringLength = 40;
const uid_t kRootUid = 0;
// Time in seconds from the final kernel log message for a call stack
// to count towards the signature of the kcrash.
-static const int kSignatureTimestampWindow = 2;
+const int kSignatureTimestampWindow = 2;
// Kernel log timestamp regular expression.
-static const std::string kTimestampRegex("^<.*>\\[\\s*(\\d+\\.\\d+)\\]");
+const char kTimestampRegex[] = "^<.*>\\[\\s*(\\d+\\.\\d+)\\]";
//
// These regular expressions enable to us capture the PC in a backtrace.
@@ -42,7 +46,7 @@
// "<0>[ 37.474699] EIP: [<790ed488>] write_breakme+0x80/0x108
// SS:ESP 0068:e9dd3efc"
//
-static const char *s_pc_regex[] = {
+const char* const kPCRegex[] = {
0,
" PC is at ([^\\+ ]+).*",
" epc\\s+:\\s+\\S+\\s+([^\\+ ]+).*", // MIPS has an exception program counter
@@ -50,18 +54,17 @@
" RIP \\[<.*>\\] ([^\\+ ]+).*", // X86_64 uses RIP for the program counter
};
-using base::FilePath;
-using base::StringPrintf;
-
-COMPILE_ASSERT(arraysize(s_pc_regex) == KernelCollector::archCount,
+COMPILE_ASSERT(arraysize(kPCRegex) == KernelCollector::kArchCount,
missing_arch_pc_regexp);
+} // namespace
+
KernelCollector::KernelCollector()
: is_enabled_(false),
ramoops_dump_path_(kDumpPath),
- records_(0) {
- // We expect crash dumps in the format of the architecture we are built for.
- arch_ = GetCompilerArch();
+ records_(0),
+ // We expect crash dumps in the format of architecture we are built for.
+ arch_(GetCompilerArch()) {
}
KernelCollector::~KernelCollector() {
@@ -80,8 +83,9 @@
// Ramoops appends a header to a crash which contains ==== followed by a
// timestamp. Ignore the header.
- pcrecpp::RE record_re("====\\d+\\.\\d+\n(.*)",
- pcrecpp::RE_Options().set_multiline(true).set_dotall(true));
+ pcrecpp::RE record_re(
+ "====\\d+\\.\\d+\n(.*)",
+ pcrecpp::RE_Options().set_multiline(true).set_dotall(true));
FilePath ramoops_record;
GetRamoopsRecordPath(&ramoops_record, current_record);
@@ -232,17 +236,16 @@
}
bool KernelCollector::Enable() {
- if (arch_ == archUnknown || arch_ >= archCount ||
- s_pc_regex[arch_] == NULL) {
+ if (arch_ == kArchUnknown || arch_ >= kArchCount || kPCRegex[arch_] == NULL) {
LOG(WARNING) << "KernelCollector does not understand this architecture";
return false;
- } else {
- FilePath ramoops_record;
- GetRamoopsRecordPath(&ramoops_record, 0);
- if (!base::PathExists(ramoops_record)) {
- LOG(WARNING) << "Kernel does not support crash dumping";
- return false;
- }
+ }
+
+ FilePath ramoops_record;
+ GetRamoopsRecordPath(&ramoops_record, 0);
+ if (!base::PathExists(ramoops_record)) {
+ LOG(WARNING) << "Kernel does not support crash dumping";
+ return false;
}
// To enable crashes, we will eventually need to set
@@ -270,7 +273,7 @@
float *last_stack_timestamp,
bool *is_watchdog_crash) {
pcrecpp::RE line_re("(.+)", pcrecpp::MULTILINE());
- pcrecpp::RE stack_trace_start_re(kTimestampRegex +
+ pcrecpp::RE stack_trace_start_re(std::string(kTimestampRegex) +
" (Call Trace|Backtrace):$");
// Match lines such as the following and grab out "function_name".
@@ -286,7 +289,7 @@
// For X86:
// <4>[ 6066.849504] [<7937bcee>] ? function_name+0x66/0x6c
//
- pcrecpp::RE stack_entry_re(kTimestampRegex +
+ pcrecpp::RE stack_entry_re(std::string(kTimestampRegex) +
"\\s+\\[<[[:xdigit:]]+>\\]" // Matches " [<7937bcee>]"
"([\\s\\?(]+)" // Matches " ? (" (ARM) or " ? " (X86)
"([^\\+ )]+)"); // Matches until delimiter reached
@@ -356,24 +359,21 @@
}
}
-enum KernelCollector::ArchKind KernelCollector::GetCompilerArch(void) {
+// static
+KernelCollector::ArchKind KernelCollector::GetCompilerArch() {
#if defined(COMPILER_GCC) && defined(ARCH_CPU_ARM_FAMILY)
- return archArm;
+ return kArchArm;
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_MIPS_FAMILY)
- return archMips;
+ return kArchMips;
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_X86_64)
- return archX86_64;
+ return kArchX86_64;
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_X86_FAMILY)
- return archX86;
+ return kArchX86;
#else
- return archUnknown;
+ return kArchUnknown;
#endif
}
-void KernelCollector::SetArch(enum ArchKind arch) {
- arch_ = arch;
-}
-
bool KernelCollector::FindCrashingFunction(
pcrecpp::StringPiece kernel_dump,
bool print_diagnostics,
@@ -382,7 +382,7 @@
float timestamp = 0;
// Use the correct regex for this architecture.
- pcrecpp::RE eip_re(kTimestampRegex + s_pc_regex[arch_],
+ pcrecpp::RE eip_re(std::string(kTimestampRegex) + kPCRegex[arch_],
pcrecpp::MULTILINE());
while (eip_re.FindAndConsume(&kernel_dump, ×tamp, crashing_function)) {
@@ -417,7 +417,7 @@
std::string *panic_message) {
// Match lines such as the following and grab out "Fatal exception"
// <0>[ 342.841135] Kernel panic - not syncing: Fatal exception
- pcrecpp::RE kernel_panic_re(kTimestampRegex +
+ pcrecpp::RE kernel_panic_re(std::string(kTimestampRegex) +
" Kernel panic[^\\:]*\\:\\s*(.*)",
pcrecpp::MULTILINE());
float timestamp = 0;
@@ -524,9 +524,7 @@
}
std::string dump_basename =
- FormatDumpBasename(kKernelExecName,
- time(NULL),
- kKernelPid);
+ FormatDumpBasename(kKernelExecName, time(NULL), kKernelPid);
FilePath kernel_crash_path = root_crash_directory.Append(
StringPrintf("%s.kcrash", dump_basename.c_str()));
diff --git a/crash_reporter/kernel_collector.h b/crash_reporter/kernel_collector.h
index a68144a..5f21747 100644
--- a/crash_reporter/kernel_collector.h
+++ b/crash_reporter/kernel_collector.h
@@ -19,13 +19,13 @@
public:
// Enumeration to specify architecture type.
enum ArchKind {
- archUnknown,
- archArm,
- archMips,
- archX86,
- archX86_64,
+ kArchUnknown,
+ kArchArm,
+ kArchMips,
+ kArchX86,
+ kArchX86_64,
- archCount // Number of architectures.
+ kArchCount // Number of architectures.
};
KernelCollector();
@@ -38,9 +38,7 @@
bool Enable();
// Returns true if the kernel collection currently enabled.
- bool IsEnabled() {
- return is_enabled_;
- }
+ bool is_enabled() const { return is_enabled_; }
// Collect any preserved kernel crash dump. Returns true if there was
// a dump (even if there were problems storing the dump), false otherwise.
@@ -52,8 +50,8 @@
bool print_diagnostics);
// Set the architecture of the crash dumps we are looking at.
- void SetArch(enum ArchKind arch);
- enum ArchKind GetArch() { return arch_; }
+ void set_arch(ArchKind arch) { arch_ = arch; }
+ ArchKind arch() const { return arch_; }
private:
friend class KernelCollectorTest;
@@ -94,15 +92,17 @@
bool print_diagnostics,
std::string *panic_message);
- // Returns the architecture kind for which we are built - enum ArchKind.
- enum ArchKind GetCompilerArch(void);
+ // Returns the architecture kind for which we are built.
+ static ArchKind GetCompilerArch();
bool is_enabled_;
base::FilePath ramoops_dump_path_;
size_t records_;
// The architecture of kernel dump strings we are working with.
- enum ArchKind arch_;
+ ArchKind arch_;
+
+ DISALLOW_COPY_AND_ASSIGN(KernelCollector);
};
#endif // CRASH_REPORTER_KERNEL_COLLECTOR_H_
diff --git a/crash_reporter/kernel_collector_test.cc b/crash_reporter/kernel_collector_test.cc
index 9e00d32..ec7ad8e 100644
--- a/crash_reporter/kernel_collector_test.cc
+++ b/crash_reporter/kernel_collector_test.cc
@@ -70,7 +70,7 @@
TEST_F(KernelCollectorTest, ComputeKernelStackSignatureBase) {
// Make sure the normal build architecture is detected
- EXPECT_TRUE(collector_.GetArch() != KernelCollector::archUnknown);
+ EXPECT_NE(KernelCollector::kArchUnknown, collector_.arch());
}
TEST_F(KernelCollectorTest, LoadPreservedDump) {
@@ -91,7 +91,7 @@
TEST_F(KernelCollectorTest, EnableMissingKernel) {
ASSERT_FALSE(collector_.Enable());
- ASSERT_FALSE(collector_.IsEnabled());
+ ASSERT_FALSE(collector_.is_enabled());
ASSERT_TRUE(FindLog(
"Kernel does not support crash dumping"));
ASSERT_EQ(s_crashes, 0);
@@ -100,7 +100,7 @@
TEST_F(KernelCollectorTest, EnableOK) {
WriteStringToFile(kcrash_file(), "");
ASSERT_TRUE(collector_.Enable());
- ASSERT_TRUE(collector_.IsEnabled());
+ ASSERT_TRUE(collector_.is_enabled());
ASSERT_TRUE(FindLog("Enabling kernel crash handling"));
ASSERT_EQ(s_crashes, 0);
}
@@ -359,7 +359,7 @@
"[<c017bfe0>] (proc_reg_write+0x88/0x9c)\n";
std::string signature;
- collector_.SetArch(KernelCollector::archArm);
+ collector_.set_arch(KernelCollector::kArchArm);
EXPECT_TRUE(
collector_.ComputeKernelStackSignature(kBugToPanic, &signature, false));
EXPECT_EQ("kernel-write_breakme-97D3E92F", signature);
@@ -418,7 +418,7 @@
"<5>[ 3378.696000] ---[ end trace 75067432f24bbc93 ]---\n";
std::string signature;
- collector_.SetArch(KernelCollector::archMips);
+ collector_.set_arch(KernelCollector::kArchMips);
EXPECT_TRUE(
collector_.ComputeKernelStackSignature(kBugToPanic, &signature, false));
EXPECT_EQ("kernel-lkdtm_do_action-5E600A6B", signature);
@@ -443,7 +443,7 @@
"<4>[ 6066.950208] [<7901b260>] no_context+0x10d/0x117\n";
std::string signature;
- collector_.SetArch(KernelCollector::archX86);
+ collector_.set_arch(KernelCollector::kArchX86);
EXPECT_TRUE(
collector_.ComputeKernelStackSignature(kBugToPanic, &signature, false));
EXPECT_EQ("kernel-ieee80211_stop_tx_ba_session-DE253569", signature);