Bug 12022 - typo in SVGTransformable.cpp introduce in r18440
Summary: typo in SVGTransformable.cpp introduce in r18440
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2006-12-28 17:46 PST by Geoffrey Garen
Modified: 2006-12-29 13:16 PST (History)
1 user (show)

See Also:

First attempt (4.91 KB, patch)
2006-12-29 04:07 PST, Rob Buis
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2006-12-28 17:46:38 PST
18440    rwlbuis             if (!(end - currTransform) > 5)

should be

if (!((end - currTransform) > 5))

instead. Also, why so much ! > instead of <= or just < (x + 1)?
Comment 1 Geoffrey Garen 2006-12-28 17:47:31 PST
Eric Seidel suggested moving much of the code into inline functions, which would have prevented this kind of error.
Comment 2 Eric Seidel (no email) 2006-12-28 18:09:04 PST
One way to do this would be to replace the current check system with this inline:

static inline bool checkString(const UChar*& currTransform, const UChar*& end, const UChar* name, int length)
    if ((end - currTransform) < length)
        return false;
    if (!memcmp(name, currTransform, sizeof(UChar) * length))
        return false;
    currTransform += length;

which would be called like this:

if (checkString(currTransform, end, {'s','c','a','l','e'}, 5)) {
    required = 1;
    optional = 1;
    type = SVGTransform::SVG_TRANSFORM_SCALE;
} else if ()...

I'm not sure that's the most elegant system, but it does at least make the code smaller and less error-prone.
Comment 3 Rob Buis 2006-12-29 04:07:43 PST
Created attachment 12101 [details]
First attempt

I like the suggestion, IMHO this code is cleaner.

Comment 4 Sam Weinig 2006-12-29 07:17:18 PST
This looks like a good change but instead of doing

+            } else if (checkString(currTransform, end, skewYDesc, 5)) {

perhaps doing something like

+            } else if (checkString(currTransform, end, skewYDesc, sizeof(skewYDesc) / sizeof(UChar))) {

could prevent error?

Comment 5 Sam Weinig 2006-12-29 13:13:05 PST
Comment on attachment 12101 [details]
First attempt

r+ if you make the changes above.
Comment 6 Rob Buis 2006-12-29 13:16:36 PST
Landed in r18479.