12022 2006-12-28 17:46:38 -0800 typo in SVGTransformable.cpp introduce in r18440 2006-12-29 13:16:36 -0800 1 1 1 Unclassified WebKit SVG 420+ Mac OS X 10.4 RESOLVED FIXED P2 Normal --- 1 ggaren webkit-unassigned rwlbuis oldest_to_newest 39144 0 ggaren 2006-12-28 17:46:38 -0800 18440 rwlbuis if (!(end - currTransform) > 5) should be if (!((end - currTransform) > 5)) instead. Also, why so much ! > instead of <= or just < (x + 1)? 39145 1 ggaren 2006-12-28 17:47:31 -0800 Eric Seidel suggested moving much of the code into inline functions, which would have prevented this kind of error. 39142 2 eric 2006-12-28 18:09:04 -0800 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. 39058 3 12101 rwlbuis 2006-12-29 04:07:43 -0800 Created attachment 12101 First attempt I like the suggestion, IMHO this code is cleaner. Cheers, Rob. 39038 4 sam 2006-12-29 07:17:18 -0800 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? 39004 5 12101 sam 2006-12-29 13:13:05 -0800 Comment on attachment 12101 First attempt r+ if you make the changes above. 38990 6 rwlbuis 2006-12-29 13:16:36 -0800 Landed in r18479. 12101 2006-12-29 04:07:43 -0800 2006-12-29 13:13:05 -0800 First attempt 12022.diff text/plain 5027 rwlbuis SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAxODQ3MykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTcgQEAKKzIwMDYtMTItMjkgIFJvYiBCdWlzICA8YnVpc0BrZGUub3JnPgorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIGh0dHA6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEyMDIyCisgICAgICAgIHR5cG8gaW4gU1ZHVHJh bnNmb3JtYWJsZS5jcHAgaW50cm9kdWNlIGluIHIxODQ0MAorCisgICAgICAgIFVzZSB0aGUgaGVs cGVyIG1ldGhvZCBwcm9wb3NlZCBieSBFcmljLgorCisgICAgICAgICoga3N2ZzIvc3ZnL1NWR1Ry YW5zZm9ybWFibGUuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Y2hlY2tTdHJpbmcpOgorICAgICAg ICAoV2ViQ29yZTo6KToKKyAgICAgICAgKFdlYkNvcmU6OlNWR1RyYW5zZm9ybWFibGU6OnBhcnNl VHJhbnNmb3JtQXR0cmlidXRlKToKKwogMjAwNi0xMi0yOSAgTmlrb2xhcyBaaW1tZXJtYW5uICA8 emltbWVybWFubkBrZGUub3JnPgogCiAgICAgICAgIEJ1aWxkIGZpeGVkLCBub3QgcmV2aWV3ZWQu IE5vdGljZWQgYnkgRGFuaWVsIE1vbGtlbnRpbi4KSW5kZXg6IFdlYkNvcmUva3N2ZzIvc3ZnL1NW R1RyYW5zZm9ybWFibGUuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUva3N2ZzIvc3ZnL1NWR1Ry YW5zZm9ybWFibGUuY3BwCShyZXZpc2lvbiAxODQ3MikKKysrIFdlYkNvcmUva3N2ZzIvc3ZnL1NW R1RyYW5zZm9ybWFibGUuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC05NSw2ICs5NSwyMyBAQCBpbnQg cGFyc2VUcmFuc2Zvcm1QYXJhbUxpc3QoY29uc3QgVUNoYXIqCiAgICAgcmV0dXJuIHJlcXVpcmVk UGFyYW1zICsgb3B0aW9uYWxQYXJhbXM7CiB9CiAKK3N0YXRpYyBpbmxpbmUgYm9vbCBjaGVja1N0 cmluZyhjb25zdCBVQ2hhciomIGN1cnJUcmFuc2Zvcm0sIGNvbnN0IFVDaGFyKiYgZW5kLCBjb25z dCBVQ2hhciogbmFtZSwgaW50IGxlbmd0aCkKK3sKKyAgICBpZiAoKGVuZCAtIGN1cnJUcmFuc2Zv cm0pIDwgKGxlbmd0aCArIDEpKQorICAgICAgICByZXR1cm4gZmFsc2U7CisgICAgaWYgKG1lbWNt cChuYW1lLCBjdXJyVHJhbnNmb3JtLCBzaXplb2YoVUNoYXIpICogbGVuZ3RoKSkKKyAgICAgICAg cmV0dXJuIGZhbHNlOworICAgIGN1cnJUcmFuc2Zvcm0gKz0gbGVuZ3RoOworICAgIHJldHVybiB0 cnVlOworfQorCitzdGF0aWMgY29uc3QgVUNoYXIgc2tld1hEZXNjW10gPSAgeydzJywnaycsJ2Un LCd3JywgJ1gnfTsKK3N0YXRpYyBjb25zdCBVQ2hhciBza2V3WURlc2NbXSA9ICB7J3MnLCdrJywn ZScsJ3cnLCAnWSd9Oworc3RhdGljIGNvbnN0IFVDaGFyIHNjYWxlRGVzY1tdID0gIHsncycsJ2Mn LCdhJywnbCcsICdlJ307CitzdGF0aWMgY29uc3QgVUNoYXIgdHJhbnNsYXRlRGVzY1tdID0gIHsn dCcsJ3InLCdhJywnbicsICdzJywgJ2wnLCAnYScsICd0JywgJ2UnfTsKK3N0YXRpYyBjb25zdCBV Q2hhciByb3RhdGVEZXNjW10gPSAgeydyJywnbycsJ3QnLCdhJywgJ3QnLCAnZSd9Oworc3RhdGlj IGNvbnN0IFVDaGFyIG1hdHJpeERlc2NbXSA9ICB7J20nLCdhJywndCcsJ3InLCAnaScsICd4J307 CisKIGJvb2wgU1ZHVHJhbnNmb3JtYWJsZTo6cGFyc2VUcmFuc2Zvcm1BdHRyaWJ1dGUoU1ZHVHJh bnNmb3JtTGlzdCogbGlzdCwgY29uc3QgQXRvbWljU3RyaW5nJiB0cmFuc2Zvcm0pCiB7CiAgICAg ZG91YmxlIHhbNl0gPSB7MCwgMCwgMCwgMCwgMCwgMH07CkBAIC0xMTAsNTMgKzEyNywyOSBAQCBi b29sIFNWR1RyYW5zZm9ybWFibGU6OnBhcnNlVHJhbnNmb3JtQXR0CiAgICAgICAgIHNraXBPcHRp b25hbFNwYWNlcyhjdXJyVHJhbnNmb3JtLCBlbmQpOwogCiAgICAgICAgIGlmICgqY3VyclRyYW5z Zm9ybSA9PSAncycpIHsKLSAgICAgICAgICAgIGlmICghKGVuZCAtIGN1cnJUcmFuc2Zvcm0pID4g NSkKLSAgICAgICAgICAgICAgICBnb3RvIGJhaWxfb3V0OwotICAgICAgICAgICAgaWYgKGN1cnJU cmFuc2Zvcm1bMV0gPT0gJ2snICYmIGN1cnJUcmFuc2Zvcm1bMl0gPT0gJ2UnICYmCi0gICAgICAg ICAgICAgICAgICBjdXJyVHJhbnNmb3JtWzNdID09ICd3JykgewotICAgICAgICAgICAgICAgIGlm IChjdXJyVHJhbnNmb3JtWzRdID09ICdYJykKLSAgICAgICAgICAgICAgICAgICAgdHlwZSA9IFNW R1RyYW5zZm9ybTo6U1ZHX1RSQU5TRk9STV9TS0VXWDsKLSAgICAgICAgICAgICAgICBlbHNlIGlm IChjdXJyVHJhbnNmb3JtWzRdID09ICdZJykKLSAgICAgICAgICAgICAgICAgICAgdHlwZSA9IFNW R1RyYW5zZm9ybTo6U1ZHX1RSQU5TRk9STV9TS0VXWTsKLSAgICAgICAgICAgICAgICBlbHNlCi0g ICAgICAgICAgICAgICAgICAgIGdvdG8gYmFpbF9vdXQ7CisgICAgICAgICAgICBpZiAoY2hlY2tT dHJpbmcoY3VyclRyYW5zZm9ybSwgZW5kLCBza2V3WERlc2MsIDUpKSB7CiAgICAgICAgICAgICAg ICAgcmVxdWlyZWQgPSAxOwogICAgICAgICAgICAgICAgIG9wdGlvbmFsID0gMDsKLSAgICAgICAg ICAgIH0gZWxzZSBpZiAoY3VyclRyYW5zZm9ybVsxXSA9PSAnYycgJiYgY3VyclRyYW5zZm9ybVsy XSA9PSAnYScgJiYKLSAgICAgICAgICAgICAgICAgIGN1cnJUcmFuc2Zvcm1bM10gPT0gJ2wnICYm IGN1cnJUcmFuc2Zvcm1bNF0gPT0gJ2UnKSB7CisgICAgICAgICAgICAgICAgdHlwZSA9IFNWR1Ry YW5zZm9ybTo6U1ZHX1RSQU5TRk9STV9TS0VXWDsKKyAgICAgICAgICAgIH0gZWxzZSBpZiAoY2hl Y2tTdHJpbmcoY3VyclRyYW5zZm9ybSwgZW5kLCBza2V3WURlc2MsIDUpKSB7CisgICAgICAgICAg ICAgICAgcmVxdWlyZWQgPSAxOworICAgICAgICAgICAgICAgIG9wdGlvbmFsID0gMDsKKyAgICAg ICAgICAgICAgICB0eXBlID0gU1ZHVHJhbnNmb3JtOjpTVkdfVFJBTlNGT1JNX1NLRVdZOworICAg ICAgICAgICAgfSBlbHNlIGlmIChjaGVja1N0cmluZyhjdXJyVHJhbnNmb3JtLCBlbmQsIHNjYWxl RGVzYywgNSkpIHsKICAgICAgICAgICAgICAgICByZXF1aXJlZCA9IDE7CiAgICAgICAgICAgICAg ICAgb3B0aW9uYWwgPSAxOwogICAgICAgICAgICAgICAgIHR5cGUgPSBTVkdUcmFuc2Zvcm06OlNW R19UUkFOU0ZPUk1fU0NBTEU7CiAgICAgICAgICAgICB9IGVsc2UKICAgICAgICAgICAgICAgICBn b3RvIGJhaWxfb3V0OwotICAgICAgICAgICAgY3VyclRyYW5zZm9ybSArPSA1OwotICAgICAgICB9 IGVsc2UgaWYgKCpjdXJyVHJhbnNmb3JtID09ICd0JykgewotICAgICAgICAgICAgaWYgKCEoKGVu ZCAtIGN1cnJUcmFuc2Zvcm0pID4gOSkgJiYKLSAgICAgICAgICAgICAgICAgIGN1cnJUcmFuc2Zv cm1bMV0gPT0gJ3InICYmIGN1cnJUcmFuc2Zvcm1bMl0gPT0gJ2EnICYmCi0gICAgICAgICAgICAg ICAgICBjdXJyVHJhbnNmb3JtWzNdID09ICduJyAmJiBjdXJyVHJhbnNmb3JtWzRdID09ICdzJyAm JgotICAgICAgICAgICAgICAgICAgY3VyclRyYW5zZm9ybVs1XSA9PSAnbCcgJiYgY3VyclRyYW5z Zm9ybVs2XSA9PSAnYScgJiYKLSAgICAgICAgICAgICAgICAgIGN1cnJUcmFuc2Zvcm1bN10gPT0g J3QnICYmIGN1cnJUcmFuc2Zvcm1bOF0gPT0gJ2UnKQotICAgICAgICAgICAgICAgIGdvdG8gYmFp bF9vdXQ7Ci0gICAgICAgICAgICBjdXJyVHJhbnNmb3JtICs9IDk7CisgICAgICAgIH0gZWxzZSBp ZiAoY2hlY2tTdHJpbmcoY3VyclRyYW5zZm9ybSwgZW5kLCB0cmFuc2xhdGVEZXNjLCA5KSkgewog ICAgICAgICAgICAgcmVxdWlyZWQgPSAxOwogICAgICAgICAgICAgb3B0aW9uYWwgPSAxOwogICAg ICAgICAgICAgdHlwZSA9IFNWR1RyYW5zZm9ybTo6U1ZHX1RSQU5TRk9STV9UUkFOU0xBVEU7Ci0g ICAgICAgIH0gZWxzZSBpZiAoKmN1cnJUcmFuc2Zvcm0gPT0gJ3InKSB7Ci0gICAgICAgICAgICBp ZiAoISgoZW5kIC0gY3VyclRyYW5zZm9ybSkgPiA2KSAmJgotICAgICAgICAgICAgICAgICAgY3Vy clRyYW5zZm9ybVsxXSA9PSAnbycgJiYgY3VyclRyYW5zZm9ybVsyXSA9PSAndCcgJiYKLSAgICAg ICAgICAgICAgICAgIGN1cnJUcmFuc2Zvcm1bM10gPT0gJ2EnICYmIGN1cnJUcmFuc2Zvcm1bNF0g PT0gJ3QnICYmCi0gICAgICAgICAgICAgICAgICBjdXJyVHJhbnNmb3JtWzVdID09ICdlJykKLSAg ICAgICAgICAgICAgICBnb3RvIGJhaWxfb3V0OwotICAgICAgICAgICAgY3VyclRyYW5zZm9ybSAr PSA2OworICAgICAgICB9IGVsc2UgaWYgKGNoZWNrU3RyaW5nKGN1cnJUcmFuc2Zvcm0sIGVuZCwg cm90YXRlRGVzYywgNikpIHsKICAgICAgICAgICAgIHJlcXVpcmVkID0gMTsKICAgICAgICAgICAg IG9wdGlvbmFsID0gMjsKICAgICAgICAgICAgIHR5cGUgPSBTVkdUcmFuc2Zvcm06OlNWR19UUkFO U0ZPUk1fUk9UQVRFOwotICAgICAgICB9IGVsc2UgaWYgKCpjdXJyVHJhbnNmb3JtID09ICdtJykg ewotICAgICAgICAgICAgaWYgKCEoKGVuZCAtIGN1cnJUcmFuc2Zvcm0pID4gNikgJiYgY3VyclRy YW5zZm9ybVsxXSA9PSAnYScgJiYKLSAgICAgICAgICAgICAgICAgIGN1cnJUcmFuc2Zvcm1bMl0g PT0gJ3QnICYmIGN1cnJUcmFuc2Zvcm1bM10gPT0gJ3InICYmCi0gICAgICAgICAgICAgICAgICBj dXJyVHJhbnNmb3JtWzRdID09ICdpJyAmJiBjdXJyVHJhbnNmb3JtWzVdID09ICd4JykKLSAgICAg ICAgICAgICAgICBnb3RvIGJhaWxfb3V0OwotICAgICAgICAgICAgY3VyclRyYW5zZm9ybSArPSA2 OworICAgICAgICB9IGVsc2UgaWYgKGNoZWNrU3RyaW5nKGN1cnJUcmFuc2Zvcm0sIGVuZCwgbWF0 cml4RGVzYywgNikpIHsKICAgICAgICAgICAgIHJlcXVpcmVkID0gNjsKICAgICAgICAgICAgIG9w dGlvbmFsID0gMDsKICAgICAgICAgICAgIHR5cGUgPSBTVkdUcmFuc2Zvcm06OlNWR19UUkFOU0ZP Uk1fTUFUUklYOwo=