SharedBufferPOSIX.cpp - SharedBuffer::m_size has to be set
Created attachment 98460 [details] Patch
Comment on attachment 98460 [details] Patch How do we test this? Is there a public function we should call instead of setting m_size directly?
Comment on attachment 98460 [details] Patch In general this looks fine though.
How did you detect that this was broken?
CCing anders who originally created the SharedBuffer class. It's strange to me that m_size is not used by most ports.
Comment on attachment 98460 [details] Patch Clearing cq+. It seems to me it would be better for this code to read into a Vector and then create the SharedBuffer from the Vector. SharedBuffer doesn't seem to be designed to be read into directly, it's meant to wrap other data storage mechanisms. Since this POSIX implementation doesn't have any "native" data storage, seems we should use Vector.
We already have a adoptVector(Vector) which gets all this right, including setting m_size. I worry that there are other things that might be missing from this implementation in the future. re-using the adoptVector(Vector) method may be more future-proof.
(In reply to comment #2) > (From update of attachment 98460 [details]) > How do we test this? Currently, there is no port in upstream that uses this file. So, we cannot test it for now. > Is there a public function we should call instead of setting m_size directly? There is no setter for m_size.
(In reply to comment #4) > How did you detect that this was broken? When I used the linux port that is mentioned in https://bugs.webkit.org/show_bug.cgi?id=39348#c6, I found this bug.
(In reply to comment #7) > We already have a adoptVector(Vector) which gets all this right, including setting m_size. I worry that there are other things that might be missing from this implementation in the future. re-using the adoptVector(Vector) method may be more future-proof. Ok. After modifying it, I'll submit new patch.
Created attachment 99961 [details] Patch
Changed to use adoptVector(Vector).
Comment on attachment 99961 [details] Patch Nice!
Comment on attachment 99961 [details] Patch Clearing flags on attachment: 99961 Committed r90574: <http://trac.webkit.org/changeset/90574>
All reviewed patches have been landed. Closing bug.