New snapshot please take a look.
http://codereview.chromium.org/8676/diff/205/8 File src/platform-win32.cc (right): http://codereview.chromium.org/8676/diff/205/8#newcode785 Line 785: _set_abort_behavior(0, _WRITE_ABORT_MSG); On 2008/10/29 07:53:43, Christian Plesner Hansen wrote: > How about making this conditional on a command-line flag -- always overriding > these seems questionable. I think it is OK to do supress the abort message. All the places in the code that calls OS::Abort() already has printed the cause of the abort, and having the MSVCRT add an additional "This application has requested the Runtime to terminate it in an unusual way. Please contact the application's support team for more information." seems weird. AFAIK Linux does not print anything when abort() is called. http://codereview.chromium.org/8676/diff/205/7 File tools/test.py (right): http://codereview.chromium.org/8676/diff/205/7#newcode175 Line 175: if len(self.crashed) > 0: On 2008/10/29 07:53:43, Christian Plesner Hansen wrote: > You never use the contents of the crashed list, only its length. You could use > a counter instead. Changed to a counter. Details on the crashed tests can be found in the failed list if we where to print details no these. http://codereview.chromium.org/8676/diff/205/7#newcode176 Line 176: print "=== %i tests CRASHED" % len(self.failed) On 2008/10/29 07:53:43, Christian Plesner Hansen wrote: > I think you mean len(self.crashed). Fixed. http://codereview.chromium.org/8676/diff/205/7#newcode404 Line 404: except: On 2008/10/29 07:53:43, Christian Plesner Hansen wrote: > Catchall exception handlers in python are extremely evil. You should only catch > the kind of exceptions you expect, in this case ImportError. Changed to catch only ImportError. http://codereview.chromium.org/8676/diff/205/7#newcode405 Line 405: Pass On 2008/10/29 07:53:43, Christian Plesner Hansen wrote: > It's 'pass', not 'Pass'. Ahem, did you test this? Fixed. I did test it when I used "None" instead of "Pass". http://codereview.chromium.org/8676/diff/205/7#newcode1075 Line 1075: result.add_option("--win-error-box", help="Display the Windows general protection fault message box", On 2008/10/29 07:53:43, Christian Plesner Hansen wrote: > How about only adding this if we're actually on windows, and then removing the > win- part of the name. Also, I think error-box is a somewhat confusing name; > maybe something like --suppress-dialogs would be better? Agree that --suppress-dialogs is a much better name. Defaults to True with the additional option --no-suppress-dialogs to turn it off. Only available on Windows. http://codereview.chromium.org/8676 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
