[Webkit-unassigned] [Bug 144046] [UNIX] Simplify the file descriptor handling in SharedMemory

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=144046

--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #2)
> Comment on attachment 251329 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=251329&action=review
> 
> Looks OK.

Thanks for the review.

> > Source/WebKit2/Platform/IPC/Attachment.h:59
> > +    Attachment(const Attachment&) = default;
> > +    Attachment& operator=(Attachment&) = default;
> 
> I think you want = delete here, not = default. It doesn’t seem OK to just
> copy a file descriptor!

Yes, I first tried to make the class non copyable, and release the fd in the destructor to get rid of the dispose() method and the explicit attachment dispose made by the ArgumentEncoder, ArgumentDecoder and ConnectionUnix. But it was not possible, or I didn't find the way. Encoding/decoding attachments is adding them to the encoder/decoder class that take the ownership of the file descriptors until they are passed to the platform specific send/receive IPC methods. The thing is that Attachment::encode() is const and does encoder.addAttachment(*this); Even if we change ArgumentEncoder::addAttachment to receive an Attachment&& we can't WTF::move(*this) in the const encode method. I'm not a C++ expert, so maybe there's a way. In the end I forgot about that part and left the explicit dispose, but avoiding the manual fd = -1 in several places and renaming the confusing adoptFrom, releaseTo methods.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150423/2d37bc9d/attachment.html>