From dcc56bf748e948579a72e484633c6c9d86b01132 Mon Sep 17 00:00:00 2001
From: Michael Lotz <mmlr@mlotz.ch>
Date: Sun, 24 Nov 2013 21:32:30 +0100
Subject: [PATCH] BNetEndpoint: Fix socket leak and reduce Accept() overhead.

When using the copy constructor of BNetEndpoint the socket of the
original endpoint gets dup'ed. The Accept() method later directly reset
the fSocket member of the newly created BNetEndpoint to the socket
returned by accept(). The socket dup'ed by the copy constructor was
therefore leaked.

Of course dup'ing the socket and copying the local and remote addresses
is superfluous in the accept case, as these members all get set to new
values. To reduce that overhead there is now a new private constructor
that directly gets the final socket and remote and local address.
---
 headers/os/net/NetEndpoint.h               |  4 ++
 src/kits/network/libnetapi/NetEndpoint.cpp | 45 +++++++++++++++-------
 2 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/headers/os/net/NetEndpoint.h b/headers/os/net/NetEndpoint.h
index 1dc3d62cad..37a0e27fd6 100644
--- a/headers/os/net/NetEndpoint.h
+++ b/headers/os/net/NetEndpoint.h
@@ -78,6 +78,10 @@ class BNetEndpoint : public BArchivable {
 		const BNetAddress& RemoteAddr();
 
 	private:
+		BNetEndpoint(const BNetEndpoint& other, int socket,
+			const struct sockaddr_in& localAddress,
+			const struct sockaddr_in& peerAddress);
+
 		status_t _SetupSocket();
 
 		virtual	void _ReservedBNetEndpointFBCCruft1();
diff --git a/src/kits/network/libnetapi/NetEndpoint.cpp b/src/kits/network/libnetapi/NetEndpoint.cpp
index e50225d96f..80bbe9b236 100644
--- a/src/kits/network/libnetapi/NetEndpoint.cpp
+++ b/src/kits/network/libnetapi/NetEndpoint.cpp
@@ -105,6 +105,23 @@ BNetEndpoint::BNetEndpoint(const BNetEndpoint& endpoint)
 }
 
 
+// Private constructor only used from BNetEndpoint::Accept().
+BNetEndpoint::BNetEndpoint(const BNetEndpoint& endpoint, int socket,
+	const struct sockaddr_in& localAddress,
+	const struct sockaddr_in& peerAddress)
+	:
+	fStatus(endpoint.fStatus),
+	fFamily(endpoint.fFamily),
+	fType(endpoint.fType),
+	fProtocol(endpoint.fProtocol),
+	fSocket(socket),
+	fTimeout(endpoint.fTimeout),
+	fAddr(localAddress),
+	fPeer(peerAddress)
+{
+}
+
+
 BNetEndpoint&
 BNetEndpoint::operator=(const BNetEndpoint& endpoint)
 {
@@ -422,33 +439,33 @@ BNetEndpoint::Accept(int32 timeout)
 	if (!IsDataPending(timeout < 0 ? B_INFINITE_TIMEOUT : 1000LL * timeout))
 		return NULL;
 
-	struct sockaddr_in addr;
-	socklen_t addrSize = sizeof(addr);
+	struct sockaddr_in peerAddress;
+	socklen_t peerAddressSize = sizeof(peerAddress);
 
-	int socket = accept(fSocket, (struct sockaddr *) &addr, &addrSize);
+	int socket
+		= accept(fSocket, (struct sockaddr *)&peerAddress, &peerAddressSize);
 	if (socket < 0) {
 		Close();
 		fStatus = errno;
 		return NULL;
 	}
 
-	BNetEndpoint* endpoint = new (std::nothrow) BNetEndpoint(*this);
+	struct sockaddr_in localAddress;
+	socklen_t localAddressSize = sizeof(localAddress);
+	if (getsockname(socket, (struct sockaddr *)&localAddress,
+			&localAddressSize) < 0) {
+		fStatus = errno;
+		return NULL;
+	}
+
+	BNetEndpoint* endpoint = new (std::nothrow) BNetEndpoint(*this, socket,
+		localAddress, peerAddress);
 	if (endpoint == NULL) {
 		close(socket);
 		fStatus = B_NO_MEMORY;
 		return NULL;
 	}
 
-	endpoint->fSocket = socket;
-	endpoint->fPeer.SetTo(addr);
-
-	if (getsockname(socket, (struct sockaddr *)&addr, &addrSize) < 0) {
-		delete endpoint;
-		fStatus = errno;
-		return NULL;
-	}
-
-	endpoint->fAddr.SetTo(addr);
 	return endpoint;
 }