[webkit-reviews] review granted: [Bug 181558] [WebGL] Simulated vertexAttrib0 can sometimes cause OUT_OF_MEMORY errors : [Attachment 331115] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 11 14:30:15 PST 2018


Eric Carlson <eric.carlson at apple.com> has granted Dean Jackson
<dino at apple.com>'s request for review:
Bug 181558: [WebGL] Simulated vertexAttrib0 can sometimes cause OUT_OF_MEMORY
errors
https://bugs.webkit.org/show_bug.cgi?id=181558

Attachment 331115: Patch

https://bugs.webkit.org/attachment.cgi?id=331115&action=review




--- Comment #3 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 331115
  --> https://bugs.webkit.org/attachment.cgi?id=331115
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=331115&action=review

> Source/WebCore/ChangeLog:13
> +	   accidentally cast a unsigned to a signed.

Nit: "a unsigned" => "an unsigned"

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1830
> +    Checked<unsigned, RecordOverflow> checkedNumElementsRequired =
Checked<unsigned>(maxIndex.value());
> +    checkedNumElementsRequired += 1;
> +    if (checkedNumElementsRequired.hasOverflowed())
> +	   return false;
> +    numElementsRequired = checkedNumElementsRequired.unsafeGet();
> +    return true;

Nit: you could create a helper function for this since you have the same code
in several places. Something like this uncompiled code might work:

    template <typename T> static inline std::optional<T> checkedAddUnsigned(T
value, T toAdd)
    {
	Checked<T, RecordOverflow> checkedResult = Checked<T>(value);
	checkedResult += toAdd;
	if (checkedResult.hasOverflowed())
	    return std::nullopt;

	return checkedResult.unsafeGet();
    }

...

    auto checkedResult = checkedAddUnsigned(maxIndex.value(), 1);
    if (!checkedResult)
	return false;

    numElementsRequired = checkedResult.value();
    return true;


More information about the webkit-reviews mailing list