[webkit-reviews] review granted: [Bug 22834] Mismatched memory free in the new CSSSelectorList : [Attachment 26766] Patch attempt.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Dean McNamee <deanm at chromium.org>'s
request for review:
Bug 22834: Mismatched memory free in the new CSSSelectorList
https://bugs.webkit.org/show_bug.cgi?id=22834

Attachment 26766: Patch attempt.
https://bugs.webkit.org/attachment.cgi?id=26766&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> -	   fastFree(selectorVector[i]);
> +	   // We want to free the memory (which was allocated with new), but we

> +	   // don't want the destructor to run since it will affect the copy
> +	   // we've just made.	In theory this is undefined, but operator
delete
> +	   // is only defined taking a void*, so in practice it should be ok.
> +	   delete reinterpret_cast<char*>(selectorVector[i]);

While this is no better than fastFree in practice, you've convinced me it’s
worthwhile because it makes valgrind stop complaining, and this makes it
slightly more likely the code would work if we made delete and fastFree
incompatible with each other.

Once this is done, we still have a bug, but I suppose it’s a less serious one
than what we have now.

> +    // We had two cases in adoptSelectVector.  The fast case of a 1 element
> +    // vector took the CSSSelector directly, which was allocated with new.
> +    // The second case we allocated a new fastMalloc buffer, which sholud be

> +    // freed with fastFree, and the destructors called manually.

Whoever lands this should fix the typo in "sholud".

We also normally do one space after a period, not two.

r=me