[webkit-reviews] review granted: [Bug 43401] REGRESSION: calling unshift passing more than 1 argument causes array corruption : [Attachment 63467] Updated patch with fix for <tabs> in source and inclusion of new regression tests.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 4 11:13:09 PDT 2010


Darin Adler <darin at apple.com> has granted Michael Saboff <msaboff at apple.com>'s
request for review:
Bug 43401: REGRESSION: calling unshift passing more than 1 argument causes
array corruption
https://bugs.webkit.org/show_bug.cgi?id=43401

Attachment 63467: Updated patch with fix for <tabs> in source and inclusion of
new regression tests.
https://bugs.webkit.org/attachment.cgi?id=63467&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Looks good. Nice test too.

> +    for (unsigned i = 0; i < (unsigned)count; i++)

We normally don't use C-style casts, so this should be a static_cast, or even
better you could just use the same type for "i" as for "count" and avoid the
cast entirely.

I’ll leave this on commit-queue? so you can decide whether to fix this. If you
choose to upload a new patch, then I'll set commit-queue+ on that, or I can set
commit-queue+ on this if you want to land it without changing the typecast
above.

Not sure why you are using shouldEvaluateTo instead of shouldBe in the test.
What's the difference?

The patch was uploaded with the wrong MIME type and without the "is a patch"
check box set; I had to fix those before I could do this review.


More information about the webkit-reviews mailing list