[webkit-reviews] review granted: [Bug 221194] Fix longstanding FIXME in parseNumericColor about not doubly clamping color components : [Attachment 419010] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 221194: Fix longstanding FIXME in parseNumericColor about not doubly
clamping color components
https://bugs.webkit.org/show_bug.cgi?id=221194

Attachment 419010: Patch

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




--- Comment #9 from Darin Adler <darin at apple.com> ---
Comment on attachment 419010
  --> https://bugs.webkit.org/attachment.cgi?id=419010
Patch

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

> Source/WebCore/css/parser/CSSParserFastPaths.cpp:317
> +    ASSERT(localValue <= 255);

I see that these functions mostly use 255, not 0xFF, nor some symbolic constant
or long horrible thing like
static_cast<int>(std::numeric_limits<uint8_t>::max()).

For the record, I slightly prefer 0xFF. But there’s no 0xFF.0 for floating
point, so maybe that’s a non-starter.

> Source/WebCore/css/parser/CSSParserFastPaths.cpp:371
> +	   uint8_t result = negative ? 0 : tenthAlphaValues[string[length - 2]
- '0'];

Could stick the character into a local variable instead of the result, if we
want to avoid having to state the type of the result.