Summary: | SharedBuffer::buffer() fails with platform data | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Avi Drissman <avi> | ||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | UNCONFIRMED --- | ||||||
Severity: | Normal | CC: | andersca, mitz | ||||
Priority: | P2 | ||||||
Version: | 525.x (Safari 3.1) | ||||||
Hardware: | Mac | ||||||
OS: | OS X 10.5 | ||||||
Attachments: |
|
Description
Avi Drissman
2008-06-20 07:58:07 PDT
Created attachment 21857 [details]
Patch to fix the buffer() call to properly use the platform data if available
Anders, do you think this is the right fix? As long as we're patching that line, we really need to move the "&" one to the left. Comment on attachment 21857 [details]
Patch to fix the buffer() call to properly use the platform data if available
The patch looks good, but I'm curious where we call buffer() on a wrapped SharedBuffer.
If this is not yet a problem, I'd be more happy with an ASSERT(!hasPlatformData()).
Do we call buffer() at all? - If not, then we shouldn't have an interface that's not used and buffer() should be dropped. - If so, then just because today we don't happen to call it while there's platform data doesn't mean tomorrow we won't. I'm not too fond of putting in an assert to catch a reasonable usage that happens to give the wrong result. Comment on attachment 21857 [details]
Patch to fix the buffer() call to properly use the platform data if available
Assigning this to Anders for review.
Comment on attachment 21857 [details]
Patch to fix the buffer() call to properly use the platform data if available
I think that we should state that calling buffer() on a SharedBuffer that wraps platform data is invalid, and add an assert.
I don't blindly want to add a maybeTransferPlatformData() because that is a potentially expensive operation and we'd like to avoid that, especially since buffer() returns a const reference.
|