Bug 12022 – typo in SVGTransformable.cpp introduce in r18440

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Geoffrey Garen]

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

should be

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

instead. Also, why so much ! > instead of <= or just < (x + 1)?

[Geoffrey Garen]

Eric Seidel suggested moving much of the code into inline functions, which would have prevented this kind of error.

[Eric Seidel (no email)]

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.

[Rob Buis]

Created attachment 12101 [details]
First attempt

I like the suggestion, IMHO this code is cleaner.
Cheers,

Rob.

[Sam Weinig]

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?

[Sam Weinig]

Comment on attachment 12101 [details]
First attempt

r+ if you make the changes above.

[Rob Buis]

Landed in r18479.