Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(8348)

Issue 4801060: Test another couple of cases. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 8 months ago by Siggi
Modified:
12 years, 8 months ago
Reviewers:
Roger McFarlane
CC:
sawbuck-changes_googlegroups.com
Base URL:
https://sawbuck.googlecode.com/svn/trunk
Visibility:
Public.

Description

Test another couple of cases. R=rogerm@chromium.org BUG=None TEST=None Committed: http://code.google.com/p/sawbuck/source/browse/#svn/trunk388

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -3 lines) Patch
M syzygy/call_trace/call_trace_dll_unittest.cc View 5 chunks +118 lines, -3 lines 2 comments Download

Messages

Total messages: 3
Siggi
12 years, 8 months ago (2011-07-28 12:43:56 UTC) #1
Roger McFarlane
lgtm http://codereview.appspot.com/4801060/diff/1/syzygy/call_trace/call_trace_dll_unittest.cc File syzygy/call_trace/call_trace_dll_unittest.cc (right): http://codereview.appspot.com/4801060/diff/1/syzygy/call_trace/call_trace_dll_unittest.cc#newcode536 syzygy/call_trace/call_trace_dll_unittest.cc:536: push edi why is it important to save ...
12 years, 8 months ago (2011-07-28 17:39:50 UTC) #2
Siggi
12 years, 8 months ago (2011-07-28 17:48:05 UTC) #3
Thanks, committing.

http://codereview.appspot.com/4801060/diff/1/syzygy/call_trace/call_trace_dll...
File syzygy/call_trace/call_trace_dll_unittest.cc (right):

http://codereview.appspot.com/4801060/diff/1/syzygy/call_trace/call_trace_dll...
syzygy/call_trace/call_trace_dll_unittest.cc:536: push edi
On 2011/07/28 17:39:50, Roger McFarlane wrote:
> why is it important to save these?

As far as I know, these are supposed to be preserved by all functions, but
there's no guarantee that the compiler won't use them in the code below.
I had some - very strange - fallout due to this in release versions of the tests
below, and better safe than sorry.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b