[webkit-reviews] review granted: [Bug 25930] Mismatched free : [Attachment 39346] updated proposed patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Zoltan Horvath <zoltan at webkit.org>'s
request for review:
Bug 25930: Mismatched free
https://bugs.webkit.org/show_bug.cgi?id=25930

Attachment 39346: updated proposed patch
https://bugs.webkit.org/attachment.cgi?id=39346&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
The name fastPureDelete isn't so clear. I think fastDeleteSkippingDestructor
would be better.

I think that "pure" is not a good term because it makes it sound like the other
delete is not as good as this one. It's this strange deletion without
destruction that needs the longer, uglier names.

> +    template <typename T>
> +    inline void fastPureDelete(T* p)

> -// Using WTF::FastAllocBase to avoid using FastAllocBase's explicit
qualification by WTF::.
> +// Avoid explicit qualification by WTF::

I don't think there should be a comment here at all. The other WTF files don't
have a comment before their using statements.

> +    template<typename ValueType, typename HashTableType>
> +    void fastDeleteAllValues(HashTableType& collection)
> +    {
> +	   typedef typename HashTableType::const_iterator iterator;
> +	   iterator end = collection.end();
> +	   for (iterator it = collection.begin(); it != end; ++it)
> +	       fastDelete(*it);
> +    }
> +
> +    template<typename T, typename U, typename V>
> +    inline void fastDeleteAllValues(const HashSet<T, U, V>& collection)
> +    {
> +	   fastDeleteAllValues<typename HashSet<T, U,
V>::ValueType>(collection.m_impl);
> +    }

It's very sad to have two versions of this because we have two versions of
delete. In the end it's not scalable to have two of everything related to new
and delete.

r=me because my only quibble is with a function name