[webkit-reviews] review denied: [Bug 119312] Replace numerous manual CRASH's in WebCore with RELEASE_ASSERT : [Attachment 207806] Patch

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has denied Kwang Yul Seo
<skyul at company100.net>'s request for review:
Bug 119312: Replace numerous manual CRASH's in WebCore with RELEASE_ASSERT
https://bugs.webkit.org/show_bug.cgi?id=119312

Attachment 207806: Patch
https://bugs.webkit.org/attachment.cgi?id=207806&action=review

------- Additional Comments from Oliver Hunt <oliver at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=207806&action=review


> Source/WebCore/dom/Text.cpp:135
> -	   if (std::numeric_limits<unsigned>::max() - data.length() <
resultLength)
> -	       CRASH();
> +	   RELEASE_ASSERT(std::numeric_limits<unsigned>::max() - data.length()
>= resultLength);

I'd rather this simply be changed to have resultLength be a Checked<> type

> Source/WebCore/platform/SharedBuffer.cpp:77
> -    if (size < 0)
> -	   CRASH();
> +    RELEASE_ASSERT(size >= 0);

Is there a reason that m_size can't be Checked<>?

> Source/WebCore/platform/audio/AudioArray.h:58
> +	   RELEASE_ASSERT(n <= std::numeric_limits<unsigned>::max() /
sizeof(T));

Again, Checked<> makes quick universally sound work of this :)