Code review: more processor logging (#82)

24 views
Skip to first unread message

Mark Mentovai

unread,
May 17, 2007, 3:38:47 PM5/17/07
to Ted Mielczarek, google-br...@googlegroups.com
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

unread,
May 17, 2007, 3:45:58 PM5/17/07
to Ted Mielczarek, google-br...@googlegroups.com
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

unread,
May 21, 2007, 3:43:17 PM5/21/07
to Mark Mentovai, google-br...@googlegroups.com
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

Reply all
Reply to author
Forward
0 new messages