[UNIX] Simplify the file descriptor handling in SharedMemory
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Apr 2015 07:28:48 +0000 (07:28 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Apr 2015 07:28:48 +0000 (07:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144046

Reviewed by Darin Adler.

Simplify the file descriptor handling and clarify its ownership by
using IPC::Attachment in SharedMemory::Handle instead of fd and
size members. SharedMemory::Handle::adoptFromAttachment() has been
renamed as SharedMemory::Handle::adoptAttachment() and receives an
IPC::Attachment. And SharedMemory::Handle::releaseToAttachment()
has been renamed as SharedMemory::Handle::releaseAttachment().

* Platform/IPC/Attachment.h: Add move constructor and move assigned operator.
* Platform/IPC/Connection.h:
(IPC::Connection::identifierIsNull): A file descriptor is null
when it's -1 no 0.
* Platform/IPC/unix/AttachmentUnix.cpp:
(IPC::Attachment::Attachment):
(IPC::Attachment::operator=):
(IPC::Attachment::dispose): Reset the file descriptor after
closing it.
* Platform/IPC/unix/ConnectionUnix.cpp:
(IPC::Connection::processMessage): Use
SharedMemory::Handle::adoptAttachment() that receives an
IPC::Attachment now.
(IPC::Connection::sendOutgoingMessage): Use
SharedMemory::Handle::releaseAttachment().
* Platform/SharedMemory.h:
* Platform/unix/SharedMemoryUnix.cpp:
(WebKit::SharedMemory::Handle::Handle): Remove initializers for
file descriptor and size members.
(WebKit::SharedMemory::Handle::clear): Dispose the attachment.
(WebKit::SharedMemory::Handle::isNull): Handle is null if the
attachment file descriptor is -1.
(WebKit::SharedMemory::Handle::encode): Use releaseAttachment().
(WebKit::SharedMemory::Handle::decode): Use adoptAttachment().
(WebKit::SharedMemory::Handle::releaseAttachment): Implement it
using move.
(WebKit::SharedMemory::Handle::adoptAttachment): Ditto.
(WebKit::SharedMemory::map): Use
IPC::Attachment::releaseFileDescriptor() instead of manually
changing the member.
(WebKit::SharedMemory::createHandle): Initialize the handle
attachment with the duplicated file descriptor and size.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@183176 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/Platform/IPC/Attachment.h
Source/WebKit2/Platform/IPC/Connection.h
Source/WebKit2/Platform/IPC/unix/AttachmentUnix.cpp
Source/WebKit2/Platform/IPC/unix/ConnectionUnix.cpp
Source/WebKit2/Platform/SharedMemory.h
Source/WebKit2/Platform/unix/SharedMemoryUnix.cpp

index 7879d95..1a0e5b7 100644 (file)
@@ -1,3 +1,50 @@
+2015-04-23  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [UNIX] Simplify the file descriptor handling in SharedMemory
+        https://bugs.webkit.org/show_bug.cgi?id=144046
+
+        Reviewed by Darin Adler.
+
+        Simplify the file descriptor handling and clarify its ownership by
+        using IPC::Attachment in SharedMemory::Handle instead of fd and
+        size members. SharedMemory::Handle::adoptFromAttachment() has been
+        renamed as SharedMemory::Handle::adoptAttachment() and receives an
+        IPC::Attachment. And SharedMemory::Handle::releaseToAttachment()
+        has been renamed as SharedMemory::Handle::releaseAttachment().
+
+        * Platform/IPC/Attachment.h: Add move constructor and move assigned operator.
+        * Platform/IPC/Connection.h:
+        (IPC::Connection::identifierIsNull): A file descriptor is null
+        when it's -1 no 0.
+        * Platform/IPC/unix/AttachmentUnix.cpp:
+        (IPC::Attachment::Attachment):
+        (IPC::Attachment::operator=):
+        (IPC::Attachment::dispose): Reset the file descriptor after
+        closing it.
+        * Platform/IPC/unix/ConnectionUnix.cpp:
+        (IPC::Connection::processMessage): Use
+        SharedMemory::Handle::adoptAttachment() that receives an
+        IPC::Attachment now.
+        (IPC::Connection::sendOutgoingMessage): Use
+        SharedMemory::Handle::releaseAttachment().
+        * Platform/SharedMemory.h:
+        * Platform/unix/SharedMemoryUnix.cpp:
+        (WebKit::SharedMemory::Handle::Handle): Remove initializers for
+        file descriptor and size members.
+        (WebKit::SharedMemory::Handle::clear): Dispose the attachment.
+        (WebKit::SharedMemory::Handle::isNull): Handle is null if the
+        attachment file descriptor is -1.
+        (WebKit::SharedMemory::Handle::encode): Use releaseAttachment().
+        (WebKit::SharedMemory::Handle::decode): Use adoptAttachment().
+        (WebKit::SharedMemory::Handle::releaseAttachment): Implement it
+        using move.
+        (WebKit::SharedMemory::Handle::adoptAttachment): Ditto.
+        (WebKit::SharedMemory::map): Use
+        IPC::Attachment::releaseFileDescriptor() instead of manually
+        changing the member.
+        (WebKit::SharedMemory::createHandle): Initialize the handle
+        attachment with the duplicated file descriptor and size.
+
 2015-04-22  Darin Adler  <darin@apple.com>
 
         Remove OwnPtr and PassOwnPtr use from WebKit/cf, WebKit/mac, and WebKit2
index b0f1dce..a6b1593 100644 (file)
@@ -53,6 +53,10 @@ public:
 #if OS(DARWIN)
     Attachment(mach_port_name_t port, mach_msg_type_name_t disposition);
 #elif USE(UNIX_DOMAIN_SOCKETS)
+    Attachment(Attachment&&);
+    Attachment& operator=(Attachment&&);
+    Attachment(const Attachment&) = default;
+    Attachment& operator=(Attachment&) = default;
     Attachment(int fileDescriptor, size_t);
     Attachment(int fileDescriptor);
 #endif
@@ -85,7 +89,7 @@ private:
     mach_port_name_t m_port;
     mach_msg_type_name_t m_disposition;
 #elif USE(UNIX_DOMAIN_SOCKETS)
-    int m_fileDescriptor;
+    int m_fileDescriptor { -1 };
     size_t m_size;
 #endif
 };
index 490316e..c6803bf 100644 (file)
@@ -129,7 +129,7 @@ public:
     pid_t remoteProcessID() const;
 #elif USE(UNIX_DOMAIN_SOCKETS)
     typedef int Identifier;
-    static bool identifierIsNull(Identifier identifier) { return !identifier; }
+    static bool identifierIsNull(Identifier identifier) { return identifier == -1; }
 
     struct SocketPair {
         int client;
index 022c72d..e67ed64 100644 (file)
@@ -45,10 +45,35 @@ Attachment::Attachment(int fileDescriptor)
 {
 }
 
+Attachment::Attachment(Attachment&& attachment)
+    : m_type(attachment.m_type)
+    , m_fileDescriptor(attachment.m_fileDescriptor)
+    , m_size(attachment.m_size)
+{
+    attachment.m_type = Uninitialized;
+    attachment.m_fileDescriptor = -1;
+    attachment.m_size = 0;
+}
+
+Attachment& Attachment::operator=(Attachment&& attachment)
+{
+    m_type = attachment.m_type;
+    attachment.m_type = Uninitialized;
+    m_fileDescriptor = attachment.m_fileDescriptor;
+    attachment.m_fileDescriptor = -1;
+    m_size = attachment.m_size;
+    attachment.m_size = 0;
+
+    return *this;
+}
+
 void Attachment::dispose()
 {
-    if (m_fileDescriptor != -1)
-        closeWithRetry(m_fileDescriptor);
+    if (m_fileDescriptor == -1)
+        return;
+
+    closeWithRetry(m_fileDescriptor);
+    m_fileDescriptor = -1;
 }
 
 } // namespace IPC
index b63882e..e88ba58 100644 (file)
@@ -247,7 +247,7 @@ bool Connection::processMessage()
         }
 
         WebKit::SharedMemory::Handle handle;
-        handle.adoptFromAttachment(m_fileDescriptors[attachmentFileDescriptorCount - 1], attachmentInfo[attachmentCount].getSize());
+        handle.adoptAttachment(IPC::Attachment(m_fileDescriptors[attachmentFileDescriptorCount - 1], attachmentInfo[attachmentCount].getSize()));
 
         oolMessageBody = WebKit::SharedMemory::map(handle, WebKit::SharedMemory::Protection::ReadOnly);
         if (!oolMessageBody) {
@@ -445,7 +445,7 @@ bool Connection::sendOutgoingMessage(std::unique_ptr<MessageEncoder> encoder)
 
         memcpy(oolMessageBody->data(), encoder->buffer(), encoder->bufferSize());
 
-        attachments.append(handle.releaseToAttachment());
+        attachments.append(handle.releaseAttachment());
     }
 
     struct msghdr message;
index 4db7452..e49fece 100644 (file)
@@ -69,17 +69,17 @@ public:
         static bool decode(IPC::ArgumentDecoder&, Handle&);
 
 #if USE(UNIX_DOMAIN_SOCKETS)
-        IPC::Attachment releaseToAttachment() const;
-        void adoptFromAttachment(int fileDescriptor, size_t);
+        IPC::Attachment releaseAttachment() const;
+        void adoptAttachment(IPC::Attachment&&);
 #endif
     private:
         friend class SharedMemory;
 #if OS(DARWIN)
         mutable mach_port_t m_port;
+        size_t m_size;
 #elif USE(UNIX_DOMAIN_SOCKETS)
-        mutable int m_fileDescriptor;
+        mutable IPC::Attachment m_attachment;
 #endif
-        size_t m_size;
     };
 
     static RefPtr<SharedMemory> allocate(size_t);
index be5f1a1..0c5a2cc 100644 (file)
@@ -47,8 +47,6 @@
 namespace WebKit {
 
 SharedMemory::Handle::Handle()
-    : m_fileDescriptor(-1)
-    , m_size(0)
 {
 }
 
@@ -59,18 +57,17 @@ SharedMemory::Handle::~Handle()
 
 void SharedMemory::Handle::clear()
 {
-    if (!isNull())
-        closeWithRetry(m_fileDescriptor);
+    m_attachment.dispose();
 }
 
 bool SharedMemory::Handle::isNull() const
 {
-    return m_fileDescriptor == -1;
+    return m_attachment.fileDescriptor() == -1;
 }
 
 void SharedMemory::Handle::encode(IPC::ArgumentEncoder& encoder) const
 {
-    encoder << releaseToAttachment();
+    encoder << releaseAttachment();
 }
 
 bool SharedMemory::Handle::decode(IPC::ArgumentDecoder& decoder, Handle& handle)
@@ -82,24 +79,21 @@ bool SharedMemory::Handle::decode(IPC::ArgumentDecoder& decoder, Handle& handle)
     if (!decoder.decode(attachment))
         return false;
 
-    handle.adoptFromAttachment(attachment.releaseFileDescriptor(), attachment.size());
+    handle.adoptAttachment(WTF::move(attachment));
     return true;
 }
 
-IPC::Attachment SharedMemory::Handle::releaseToAttachment() const
+IPC::Attachment SharedMemory::Handle::releaseAttachment() const
 {
-    int temp = m_fileDescriptor;
-    m_fileDescriptor = -1;
-    return IPC::Attachment(temp, m_size);
+    ASSERT(!isNull());
+    return WTF::move(m_attachment);
 }
 
-void SharedMemory::Handle::adoptFromAttachment(int fileDescriptor, size_t size)
+void SharedMemory::Handle::adoptAttachment(IPC::Attachment&& attachment)
 {
-    ASSERT(!m_size);
     ASSERT(isNull());
 
-    m_fileDescriptor = fileDescriptor;
-    m_size = size;
+    m_attachment = WTF::move(attachment);
 }
 
 RefPtr<SharedMemory> SharedMemory::allocate(size_t size)
@@ -161,15 +155,14 @@ RefPtr<SharedMemory> SharedMemory::map(const Handle& handle, Protection protecti
 {
     ASSERT(!handle.isNull());
 
-    void* data = mmap(0, handle.m_size, accessModeMMap(protection), MAP_SHARED, handle.m_fileDescriptor, 0);
+    void* data = mmap(0, handle.m_attachment.size(), accessModeMMap(protection), MAP_SHARED, handle.m_attachment.fileDescriptor(), 0);
     if (data == MAP_FAILED)
-        return 0;
+        return nullptr;
 
     RefPtr<SharedMemory> instance = adoptRef(new SharedMemory());
     instance->m_data = data;
-    instance->m_fileDescriptor = handle.m_fileDescriptor;
-    instance->m_size = handle.m_size;
-    handle.m_fileDescriptor = -1;
+    instance->m_fileDescriptor = handle.m_attachment.releaseFileDescriptor();
+    instance->m_size = handle.m_attachment.size();
     return instance;
 }
 
@@ -202,8 +195,7 @@ bool SharedMemory::createHandle(Handle& handle, Protection)
             return false;
         }
     }
-    handle.m_fileDescriptor = duplicatedHandle;
-    handle.m_size = m_size;
+    handle.m_attachment = IPC::Attachment(duplicatedHandle, m_size);
     return true;
 }