[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 #5 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #3)
> (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.

Zan suggested to make the copy constructor and assignment private and use it only in ::encode() doing encoder.addAttachment(Attachment(*this)); That way we can ensure that any public assignment is a move. We also need to make the file descriptor member mutable and explicitly set to -1 in ::encode() since the ownership is transfered to the encoder. It's tricky, but it works. I'll submit a new bug.

-- 
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/f84e5064/attachment.html>