[webkit-reviews] review denied: [Bug 102828] r134080 causes heap problem on linux systems where PAGESIZE != 4096 : [Attachment 181941] proposed patch.

[bugzilla-daemon at webkit.org]

Mark Hahnenberg <mhahnenberg at apple.com> has denied Balazs Kilvady
<kilvadyb at homejinni.com>'s request for review:
Bug 102828: r134080 causes heap problem on linux systems where PAGESIZE != 4096
https://bugs.webkit.org/show_bug.cgi?id=102828

Attachment 181941: proposed patch.
https://bugs.webkit.org/attachment.cgi?id=181941&action=review

------- Additional Comments from Mark Hahnenberg <mhahnenberg at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=181941&action=review


> Source/JavaScriptCore/heap/BaseBlock.h:33
> +class BaseBlock {

It seems like major overkill to define an entirely new class whose sole purpose
is to make sure a static constant is the same between two classes. Why not just
use a COMPILE_ASSERT? And why not just change the value in Options.cpp to be 4
KB rather than pageSize()?

> Source/JavaScriptCore/heap/MarkStack.cpp:-54
> -    ASSERT(MarkStackSegment::blockSize == WeakBlock::blockSize);

Why wasn't this ASSERT firing before if the MarkStackSegment blockSize was not
equal to the WeakBlock blockSize?