If you get rid of the bit fiddling and start using the IsSmi operations instead, this LGTM.
http://codereview.chromium.org/155635/diff/8/1010 File include/v8.h (right): http://codereview.chromium.org/155635/diff/8/1010#newcode1118 Line 1118: void SetPointerInInternalField(int index, void* ptr); ptr -> value? http://codereview.chromium.org/155635/diff/8/1011 File src/api.cc (right): http://codereview.chromium.org/155635/diff/8/1011#newcode2457 Line 2457: if (reinterpret_cast<uintptr_t>(pointer) & 0x1 == 0) { Why not just say: if (pointer->IsSmi())? http://codereview.chromium.org/155635/diff/8/1011#newcode2462 Line 2462: // Read from uninitialzied field There's also a chance of getting a non-proxy object out. You need to deal with that instead of checking for undefined. I would say if (!pointer->IsProxy()) return NULL; http://codereview.chromium.org/155635/diff/8/1011#newcode2462 Line 2462: // Read from uninitialzied field uninitialzied -> uninitialized. http://codereview.chromium.org/155635/diff/8/1011#newcode2484 Line 2484: void v8::Object::SetPointerInInternalField(int index, void* ptr) { Rename ptr to value? http://codereview.chromium.org/155635/diff/8/1011#newcode2486 Line 2486: if (reinterpret_cast<uintptr_t>(ptr) & 0x1 == 0) { How about if (reinterpret_cast<Object*>(ptr)->IsSmi())? http://codereview.chromium.org/155635/diff/8/1011#newcode2487 Line 2487: // Aligned pointer, store as is Terminate comment with . http://codereview.chromium.org/155635/diff/8/1011#newcode2491 Line 2491: ASSERT(reinterpret_cast<uintptr_t>(proxy) & 0x1 == 1); ASSERT(!proxy->IsSmi())? Maybe not really needed though. http://codereview.chromium.org/155635/diff/8/1012 File test/cctest/test-api.cc (right): http://codereview.chromium.org/155635/diff/8/1012#newcode1297 Line 1297: delete [] data; delete [] -> delete[] http://codereview.chromium.org/155635 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
