Thanks for this. It needs a few fixes before we can commit it.
http://codereview.chromium.org/545125/diff/1/10 File src/platform-solaris.cc (right): http://codereview.chromium.org/545125/diff/1/10#newcode29 src/platform-solaris.cc:29: // parts the implementation is in platform-posix.cc. Please add a comment to the effect that this doesn't work on SPARC. If you can add a #error then that is even better, but perhaps that's hard to test. http://codereview.chromium.org/545125/diff/1/10#newcode76 src/platform-solaris.cc:76: int isgreater(double x, double y) { We don't seem to use this anywhere. It should be removed. http://codereview.chromium.org/545125/diff/1/10#newcode81 src/platform-solaris.cc:81: // Classify floating point number - usually defined in math.h#ifndef fpclassify The line above looks wrong. http://codereview.chromium.org/545125/diff/1/10#newcode125 src/platform-solaris.cc:125: // TODO: Test to see if ceil() is correct on Solaris. Please remove this TODO. If you suspect the unit tests are insufficient on this point then please file a bug in the issue database on http://code.google.com/p/v8 http://codereview.chromium.org/545125/diff/1/10#newcode163 src/platform-solaris.cc:163: return tzname[0]; // the location of the timezone string on Solaris Comments should start with a capital letter and end with a comment. Also, please install cpplint.py from Google and run the tools/presubmit.py to catch lint errors (in this case there's a missing space. http://codereview.chromium.org/545125/diff/1/10#newcode176 src/platform-solaris.cc:176: days = loc->tm_yday = utc->tm_yday; Should this '=' be a '-'? http://codereview.chromium.org/545125/diff/1/10#newcode177 src/platform-solaris.cc:177: hours = ((days < -1 ? 24 : 1 < days ? -24 : days * 24) + I'm not sure what the line above is trying to do, but it looks like it has sign issues at the very least. http://codereview.chromium.org/545125/diff/1/10#newcode212 src/platform-solaris.cc:212: return (size_t)getpagesize(); We only allow C++-style casts, not C-style casts. http://codereview.chromium.org/545125/diff/1/10#newcode265 src/platform-solaris.cc:265: // Redirect to std abort to signal abnormal program termination Missing full stop. http://codereview.chromium.org/545125/diff/1/10#newcode311 src/platform-solaris.cc:311: UNIMPLEMENTED(); Please provide an empty implementation instead, so we can at least have logging on Solaris, even if profiling doesn't give correct results for ticks in shared libraries. http://codereview.chromium.org/545125/diff/1/10#newcode612 src/platform-solaris.cc:612: TickSample sample; Please set the sample.pc, sample.sp and sample.fp to dummy values (eg 0) and file a bug to say that profiling doesn't work on Solaris. http://codereview.chromium.org/545125/diff/1/12 File test/cctest/test-compiler.cc (right): http://codereview.chromium.org/545125/diff/1/12#newcode77 test/cctest/test-compiler.cc:77: printf("%lc", (wint_t)string[j]); C-style cast. http://codereview.chromium.org/545125
-- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev
