LGTM (with a few comments about the printed output and the usual nits).
https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py File tools/grokdump.py (right): https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newcode36 tools/grokdump.py:36: import disasm I know you didn't touch, but can we alpha-sort the imports? https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newcode709 tools/grokdump.py:709: result = re.match(r"^FUNC ([a-f0-9]+) ([a-f0-9]+) ([a-f0-9]+) (.*)$", line) More than 80 characters. https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newcode714 tools/grokdump.py:714: bisect.insort_left(self.symbols, FuncSymbol(baseaddr + start, size, name)) More than 80 characters. https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newcode1770 tools/grokdump.py:1770: print Can we move this empty print to after the modules are printed? https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newcode1771 tools/grokdump.py:1771: print " Modules:" Lowercase "modules:" would be consistent with the rest of the exception info. https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newcode1789 tools/grokdump.py:1789: print reader.FindSymbol(reader.ExceptionIP()) This will print "None" when no symbol is found, can we skip printing in that case? I think the "None" in the output is confusing. https://chromiumcodereview.appspot.com/10923003/diff/1/tools/grokdump.py#newcode1824 tools/grokdump.py:1824: maybe_symbol) Just use "maybe_symbol or ''" here and drop the if statement above. https://chromiumcodereview.appspot.com/10923003/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
