Summary: | Replace numerous manual CRASH's in WebCore with RELEASE_ASSERT | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kwang Yul Seo <skyul> | ||||||
Component: | WebCore Misc. | Assignee: | Kwang Yul Seo <skyul> | ||||||
Status: | NEW --- | ||||||||
Severity: | Normal | CC: | benjamin, commit-queue, eric.carlson, esprehn+autocc, glenn, japhet, jer.noble, kangil.han, kondapallykalyan, oliver | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Kwang Yul Seo
2013-07-30 21:08:28 PDT
Created attachment 207806 [details]
Patch
Comment on attachment 207806 [details] Patch 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 :) (In reply to comment #2) > > 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 I filed a separate bug for this. See Bug 119327. > > 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<>? The assertion here is about size, not m_size because size is signed int and m_size is unsigned int. As mentioned in the FIXME comment above, some time we should change the type of size to unsigned. > > 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 :) Also done in Bug 119327. Created attachment 207814 [details]
Patch
Comment on attachment 207814 [details]
Patch
Assuming that patches for review since 2013 are stale, r-
|