Only allow non-null pointers in the WeakSet
Created attachment 139326 [details] Patch
Comment on attachment 139326 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139326&action=review > Source/JavaScriptCore/heap/PassWeak.h:127 > template<typename T> inline PassWeak<T>::PassWeak(JSGlobalData& globalData, typename PassWeak<T>::GetType getType, WeakHandleOwner* weakOwner, void* context) > - : m_impl(globalData.heap.weakSet()->allocate(getType, weakOwner, context)) > + : m_impl(0) > { > + if (!getType) > + return; > + m_impl = globalData.heap.weakSet()->allocate(getType, weakOwner, context); > } As with your prior WebCore patch, I think there are two other ways you could do this: 1) A helper function that returns 0 if getType is 0. 2) A trinary expression getType ? xxx : 0. The “initialize to 0 and then use an early exit” doesn’t seem quite as good as those. > Source/JavaScriptCore/heap/WeakBlock.cpp:109 > const JSValue& jsValue = weakImpl->jsValue(); > - if (!jsValue || !jsValue.isCell()) > - continue; > - > JSCell* jsCell = jsValue.asCell(); > if (Heap::isMarked(jsCell)) > continue; Why bother with all these local variables? I like this: if (Heap::isMarked(weakImpl->jsValue().asCell())) > Source/JavaScriptCore/heap/WeakBlock.cpp:136 > const JSValue& jsValue = weakImpl->jsValue(); > - if (!jsValue || !jsValue.isCell()) > - continue; > - > JSCell* jsCell = jsValue.asCell(); > if (Heap::isMarked(jsCell)) > continue; Why bother with all these local variables? I like this: if (Heap::isMarked(weakImpl->jsValue().asCell()))
> As with your prior WebCore patch, I think there are two other ways you could do this: > > 1) A helper function that returns 0 if getType is 0. > 2) A trinary expression getType ? xxx : 0. OK, I did 2. > Why bother with all these local variables? I like this: I kept jsValue in one function, since it's reused later, but removed all the other locals.
Committed r115534: <http://trac.webkit.org/changeset/115534>