<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [UNIX] Simplify the file descriptor handling in SharedMemory"
href="https://bugs.webkit.org/show_bug.cgi?id=144046#c3">Comment # 3</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [UNIX] Simplify the file descriptor handling in SharedMemory"
href="https://bugs.webkit.org/show_bug.cgi?id=144046">bug 144046</a>
from <span class="vcard"><a class="email" href="mailto:cgarcia@igalia.com" title="Carlos Garcia Campos <cgarcia@igalia.com>"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=144046#c2">comment #2</a>)
<span class="quote">> Comment on <span class=""><a href="attachment.cgi?id=251329&action=diff" name="attach_251329" title="Patch">attachment 251329</a> <a href="attachment.cgi?id=251329&action=edit" title="Patch">[details]</a></span>
> Patch
>
> View in context:
> <a href="https://bugs.webkit.org/attachment.cgi?id=251329&action=review">https://bugs.webkit.org/attachment.cgi?id=251329&action=review</a>
>
> Looks OK.</span >
Thanks for the review.
<span class="quote">> > 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!</span >
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.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>