[webkit-reviews] review denied: [Bug 46066] SVG: Make SVGLength's stringToLengthType() stricter and faster : [Attachment 68733] Proposed patch v2

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Andreas Kling
<andreas.kling at nokia.com>'s request for review:
Bug 46066: SVG: Make SVGLength's stringToLengthType() stricter and faster
https://bugs.webkit.org/show_bug.cgi?id=46066

Attachment 68733: Proposed patch v2
https://bugs.webkit.org/attachment.cgi?id=68733&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=68733&action=review

The code is not realy faster with the mentioned changes, but looks more like
the rest of the parsing code in SVGParserUttilities.cpp. And it may make it
easier to change the code to ignore whitespaces at the end without parsing
error later (the default behavior on Opera, but not specified yet) by just
adding two more lines. The test looks great! Great patch!

> WebCore/svg/SVGLength.cpp:89
> +inline SVGLengthType stringToLengthType(const UChar* characters, const
UChar* end)

const UChar*& ptr

> WebCore/svg/SVGLength.cpp:94
> +    unsigned length = end - characters;
> +
> +    if (!length)
> +	   return LengthTypeNumber;

if (ptr == end)
 return LengthTypeNumber;

> WebCore/svg/SVGLength.cpp:105
> +    if (length != 1 && length != 2)
> +	   return LengthTypeUnknown;
> +
> +    const UChar lastChar = characters[length - 1];
> +
> +    if (length == 1) {
> +	   if (lastChar == '%')
> +	       return LengthTypePercentage;
> +	   return LengthTypeUnknown;
> +    }

const UChar firstChar = *ptr;
++ptr;
if (firstChar == '%') {
  if (ptr == end)
    return LengthTypePercentage;
  return LengthTypeUnkown;
}

const UChar secondChar = *ptr;
if (++ptr != end)
  return LengthTypeUnkown;