LGTM We need to update the XCode project as well.
http://codereview.chromium.org/27355/diff/17/23 File src/SConscript (right): http://codereview.chromium.org/27355/diff/17/23#newcode43 Line 43: 'flags.cc', 'frames.cc', 'global-handles.cc', 'handles.cc', 'hashmap.cc', Something is fishy about the indentation here. http://codereview.chromium.org/27355/diff/17/26 File src/debug-agent.cc (right): http://codereview.chromium.org/27355/diff/17/26#newcode50 Line 50: // Listen for new connections Add periods to the end of these comments? http://codereview.chromium.org/27355/diff/17/26#newcode72 Line 72: client->Send("Remote debugging session already active\r\n", 41); Create a const char* and use strlen? Also, why is the string terminated by \r\n instead of just \n? http://codereview.chromium.org/27355/diff/17/26#newcode132 Line 132: v8::Debug::SendCommand((const uint16_t *) temp, len); Use C++ style cast. http://codereview.chromium.org/27355/diff/17/26#newcode174 Line 174: header_buffer[header_buffer_position] = '\0'; Don't you need an if here instead? Can't you end up writing a '\0' right after the allocated buffer? http://codereview.chromium.org/27355/diff/17/26#newcode186 Line 186: // Check that key is Content-Length. Where are you checking that the key is Content-Length? http://codereview.chromium.org/27355/diff/17/26#newcode189 Line 189: content_length = 10 * content_length + (value[i] - '0'); Can there be no overflow here? http://codereview.chromium.org/27355/diff/17/27 File src/debug-agent.h (right): http://codereview.chromium.org/27355/diff/17/27#newcode41 Line 41: // Debugger agent which starts a socket listener on the debugger port and a socket -> a socket two spaces instead of one. http://codereview.chromium.org/27355/diff/17/27#newcode48 Line 48: virtual ~DebuggerAgent() {} Do you need the virtual destructor? The DebuggerAgentSession class does not have one. http://codereview.chromium.org/27355/diff/17/27#newcode72 Line 72: // Debugger agent session. The session received requests from the remote received -> receives http://codereview.chromium.org/27355/diff/17/22 File src/flag-definitions.h (right): http://codereview.chromium.org/27355/diff/17/22#newcode204 Line 204: DEFINE_bool(regexp_native, true, "use native code regexp implementation (IA32" I would break this differently: DEFINE_bool(regexp_native, true, "use native ...") ? http://codereview.chromium.org/27355 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
