http://codereview.chromium.org/2092007/diff/1/2 File src/platform-solaris.cc (right):
http://codereview.chromium.org/2092007/diff/1/2#newcode65 src/platform-solaris.cc:65: return x < 0; This will return 0 for a 'negative NaN'. That seems to be OK, since everywhere we use signbit we know it is not a NaN, but it at least deserves a comment. Alternatively i could be fixed. if (x < x) will tell you if you have a NaN, I think. signbit is part of C99 so presumably it is coming to Solaris at some point. Does the ifndef do anything? http://codereview.chromium.org/2092007/diff/1/2#newcode248 src/platform-solaris.cc:248: struct stack_walker { Unless this name is constrained by Solaris, which seems unlikely, it should be NamedLikeThis and not_like_this. http://codereview.chromium.org/2092007/diff/1/2#newcode249 src/platform-solaris.cc:249: Vector<OS::StackFrame> &frames; Incorrect placement of & (see below). http://codereview.chromium.org/2092007/diff/1/2#newcode254 src/platform-solaris.cc:254: static int StackWalkCallback(uintptr_t pc, int signo, void *data) { Incorrect placement of asterisk. The Google C++ style guide requires us to be consistent and AFAIR prefers asterisk next to the type rather than the variable. That is what we try to stick to in V8 (even though it is wrong :-). http://codereview.chromium.org/2092007/diff/1/2#newcode255 src/platform-solaris.cc:255: struct stack_walker * walker = static_cast<struct stack_walker *>(data); No space before * in types (twice in this line). http://codereview.chromium.org/2092007/diff/1/2#newcode260 src/platform-solaris.cc:260: walker->frames[i].address = (void*)pc; We don't allow C style casts. You have to use static_cast, dynamic_cast or const_cast. http://codereview.chromium.org/2092007/diff/1/2#newcode268 src/platform-solaris.cc:268: if (dladdr((void*)pc, &info) == 0) { C style cast. http://codereview.chromium.org/2092007/diff/1/2#newcode271 src/platform-solaris.cc:271: // we have containing symbol info Comments should start with a capital letter and end with a full stop (period). http://codereview.chromium.org/2092007/diff/1/2#newcode274 src/platform-solaris.cc:274: // no local symbol info Comment formatting. http://codereview.chromium.org/2092007/diff/1/2#newcode278 src/platform-solaris.cc:278: (unsigned long)pc - (unsigned long)info.dli_fbase, C style casts and %p seems inappropriate for an unsigned long. http://codereview.chromium.org/2092007/diff/1/2#newcode292 src/platform-solaris.cc:292: if (!walkcontext(&ctx, StackWalkCallback, (void*)(&walker))) { C style cast. http://codereview.chromium.org/2092007/diff/1/3 File src/platform.h (right): http://codereview.chromium.org/2092007/diff/1/3#newcode87 src/platform.h:87: # if !defined(signbit) ifndef? http://codereview.chromium.org/2092007/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
