Re: Start migrating profiles processing to C++. (issue910002)

16 views
Skip to first unread message

fin...@chromium.org

unread,
Mar 25, 2011, 11:18:48 AM3/25/11
to mnag...@chromium.org, sgj...@chromium.org, v8-...@googlegroups.com

http://codereview.chromium.org/910002/diff/10001/src/profile-generator.cc
File src/profile-generator.cc (right):

http://codereview.chromium.org/910002/diff/10001/src/profile-generator.cc#newcode143
src/profile-generator.cc:143: callback->AfterChildTraversed(parent.node,
current.node);
If the callback is a DeleteNodesCallback, then we're passing a deleted
node here into AfterChildTraversed due to line 140, no? I don't know how
this code works, can that situation occur? (CID: 9378)

http://codereview.chromium.org/910002/

mnag...@chromium.org

unread,
Mar 28, 2011, 4:16:37 AM3/28/11
to sgj...@chromium.org, fin...@chromium.org, v8-...@googlegroups.com

http://codereview.chromium.org/910002/diff/10001/src/profile-generator.cc#newcode143
src/profile-generator.cc:143: callback->AfterChildTraversed(parent.node,
current.node);

On 2011/03/25 15:18:48, Finnur wrote:
> If the callback is a DeleteNodesCallback, then we're passing a deleted
node here
> into AfterChildTraversed due to line 140, no? I don't know how this
code works,
> can that situation occur? (CID: 9378)

You are right. But in DeleteNodesCallback, AfterChildTraversed is empty,
so passing a stale pointer to it doesn't cause any harm. But I should
add a comment about it, I think. Thanks for noticing!

http://codereview.chromium.org/910002/

sgj...@chromium.org

unread,
Mar 28, 2011, 5:24:03 AM3/28/11
to mnag...@chromium.org, fin...@chromium.org, v8-...@googlegroups.com

Mikhail Naganov

unread,
Mar 28, 2011, 5:56:11 AM3/28/11
to mnag...@chromium.org, sgj...@chromium.org, fin...@chromium.org, v8-...@googlegroups.com
Soeren, this issue was committed long ago ;) This is a late drive-by ;)

On Mon, Mar 28, 2011 at 13:24, <sgj...@chromium.org> wrote:
> LGTM
>
> http://codereview.chromium.org/910002/
>

Reply all
Reply to author
Forward
0 new messages