Code review: more processor logging (#82)

[Mark Mentovai]

Ted-

I've got some logging messages I'd like you to review:

http://code.google.com/p/google-breakpad/issues/detail?id=82
http://code.google.com/p/google-breakpad/issues/attachment?aid=5273659761331288658&name=breakpad.82.3.patch

The second patch adds logging messages to the rest of the processor (I
just added them for minidump.cc and minidump_processor.cc).

Mark

[Mark Mentovai]

I changed a BPLOG in RangeMap::StoreRange to BPLOG_IF as explained in
the comment:

if (size <= 0 || high < base) {
// The processor will hit this case too frequently with common symbol
// files in the size == 0 case, which is more suited to a DEBUG channel.
// Filter those out since there's no DEBUG channel at the moment.
BPLOG_IF(INFO, size != 0) << "StoreRange failed, " << HexString(base) <<
"+" << HexString(size) << ", " <<
HexString(high);
}

[Ted Mielczarek]

Mark,

Looks good, just two comments:

Index: src/processor/basic_code_modules.cc
+ if (!map_->RetrieveRange(address, &module, NULL, NULL)) {
+ BPLOG(INFO) << "No module at " << HexString(address);
+ HexString(address);

Looks like you've got an extra HexString(address); here.

Index: src/processor/simple_symbol_supplier.cc
+ if (debug_file_name.empty()) {
+ BPLOG(ERROR) << "Can't construct symbol file path without debug_file "
+ "(code_file = " << module->code_file() << ")";

Is module->code_file() just the filename, or the full path? If the
latter, it ought to be run through PathnameStripper::File.

-Ted