Updated the xcode project using xcodebodge.py from the Chromium project - hope it works.
On 2009/03/03 11:56:14, Søren Gjesse wrote: > 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', > On 2009/03/03 11:04:31, Mads Ager wrote: > > Something is fishy about the indentation here. > Fixed (some tabs sneaked in). > 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 > On 2009/03/03 11:04:31, Mads Ager wrote: > > Add periods to the end of these comments? > Done. > http://codereview.chromium.org/27355/diff/17/26#newcode72 > Line 72: client->Send("Remote debugging session already active\r\n", 41); > On 2009/03/03 11:04:31, Mads Ager wrote: > > Create a const char* and use strlen? > > > > Also, why is the string terminated by \r\n instead of just \n? > Added static const char* message = ... and removed \r. > http://codereview.chromium.org/27355/diff/17/26#newcode132 > Line 132: v8::Debug::SendCommand((const uint16_t *) temp, len); > On 2009/03/03 11:04:31, Mads Ager wrote: > > Use C++ style cast. > Done. > http://codereview.chromium.org/27355/diff/17/26#newcode174 > Line 174: header_buffer[header_buffer_position] = '\0'; > On 2009/03/03 11:04:31, Mads Ager wrote: > > Don't you need an if here instead? Can't you end up writing a '\0' right > after > > the allocated buffer? > Added an if and a '- 1' where filling the buffer to always leave room for the > '\0'. Kept the ASSERT. > http://codereview.chromium.org/27355/diff/17/26#newcode186 > Line 186: // Check that key is Content-Length. > On 2009/03/03 11:04:31, Mads Ager wrote: > > Where are you checking that the key is Content-Length? > I was not - added this check. > http://codereview.chromium.org/27355/diff/17/26#newcode189 > Line 189: content_length = 10 * content_length + (value[i] - '0'); > On 2009/03/03 11:04:31, Mads Ager wrote: > > Can there be no overflow here? > Added range checking and bailout for non numeric data. > 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 > On 2009/03/03 11:04:31, Mads Ager wrote: > > a socket -> a socket > > > > two spaces instead of one. > Done. > http://codereview.chromium.org/27355/diff/17/27#newcode48 > Line 48: virtual ~DebuggerAgent() {} > On 2009/03/03 11:04:31, Mads Ager wrote: > > Do you need the virtual destructor? The DebuggerAgentSession class does not > > have one. > No, removed. > http://codereview.chromium.org/27355/diff/17/27#newcode72 > Line 72: // Debugger agent session. The session received requests from the > remote > On 2009/03/03 11:04:31, Mads Ager wrote: > > received -> receives > Done. > 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" > On 2009/03/03 11:04:31, Mads Ager wrote: > > I would break this differently: > > > > DEFINE_bool(regexp_native, true, > > "use native ...") > > > > ? > > > So would I - done. http://codereview.chromium.org/27355 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
