[webkit-reviews] review granted: [Bug 135069] Sharing SharedBuffer between WebCore and ImageIO is racy and crash prone : [Attachment 235345] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jul 23 10:22:25 PDT 2014
Simon Fraser (smfr) <simon.fraser at apple.com> has granted Pratik Solanki
<psolanki at apple.com>'s request for review:
Bug 135069: Sharing SharedBuffer between WebCore and ImageIO is racy and crash
prone
https://bugs.webkit.org/show_bug.cgi?id=135069
Attachment 235345: Patch
https://bugs.webkit.org/attachment.cgi?id=235345&action=review
------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=235345&action=review
> Source/WebCore/platform/SharedBuffer.cpp:424
> + DataBuffer* newBuffer = new DataBuffer;
We normally avoid bare "new" calls; I think you should wrap this in a local
RefPtr. and then do m_buffer = newBuffer.release() at the end.
> Source/WebCore/platform/SharedBuffer.h:177
> + struct DataBuffer : public RefCounted<DataBuffer> {
Does this need to be ThreadSafeRefCounted?
> Source/WebCore/platform/mac/SharedBufferMac.mm:111
> + return adoptCF((CFDataRef)adoptNS([[WebCoreSharedBufferData alloc]
initWithSharedBufferDataBuffer:copiedBuffer.get()]).leakRef());
That's a lot of adopting! I don't get why the intermediate adoptNS() is useful.
More information about the webkit-reviews
mailing list