Fix unreported glooxwrapper leaks following 61261d14fc, refs #2305.

Fixes an occurring leak indicated by the reported clang unused variable
compiler warning, refs #5294, #5550,
by adding the missing glooxwrapper::Jingle::Session::Session destructor
.

Fix two leaks that would have occurred if the according code had been
used:
Delete unused glooxwrapper::Jingle::ICEUDP::ICEUDP instead of adding the
missing destructor.
Delete unused glooxwrapper::Jingle::Content::Content instead of adding
the missing destructor.

Explain why glooxwrapper::Client::registerStanzaExtension doesn't leak
the new StanzaExtensionWrapper.
Explain why glooxwrapper::Jingle::Session::sessionInitiate doesn't leak
the new gloox::Jingle::Content, nor the new gloox::Jingle::ICEUDP.
Explain why glooxwrapper::SessionManager::registerPlugins doesn't leak
the new gloox::Jingle::Content and new gloox::Jingle::ICEUDP.
Explain why glooxwrapper::SessionManager::createSession doesn't leak the
gloox::Jingle::Session.

I will not leak memory in the glooxwrapper.
I will not leak memory in the glooxwrapper.
I will not leak memory in the glooxwrapper.

Use references in the StunClient and glooxwrapper to anticipate any
confusion as to whose obligation it is to delete variables when they are
passed around across several files.
Use static_cast and reinterpret_cast instead of C-style casts in the
StunClient.

Differential Revision: https://code.wildfiregames.com/D2094
Refs D2093 for the reported leaks.
Reviewed By: Josh
Comments By: fcxSanya, Vladislav for D2094, and echotangoecho, leper in
61261d14fc

This was SVN commit r22678.

binaries/data/mods/public/gui/gamesetup_mp/gamesetup_mp.js

@@ -100,7 +100,7 @@ function confirmSetup()
 		let joinServer = Engine.GetGUIObjectByName("joinServer").caption;
 		let joinPort = Engine.GetGUIObjectByName("joinPort").caption;
 
-		if (startJoin(joinPlayerName, joinServer, getValidPort(joinPort), false))
+		if (startJoin(joinPlayerName, joinServer, getValidPort(joinPort), false, ""))
 			switchSetupPage("pageConnecting");
 	}
 	else if (!Engine.GetGUIObjectByName("pageHost").hidden)
@@ -337,7 +337,7 @@ function startHost(playername, servername, port)
 /**
  * Connects via STUN if the hostJID is given.
  */
-function startJoin(playername, ip, port, useSTUN, hostJID = "")
+function startJoin(playername, ip, port, useSTUN, hostJID)
 {
 	try
 	{

source/lobby/IXmppClient.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2018 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -58,7 +58,7 @@ public:
 	virtual JS::Value GuiPollNewMessage(const ScriptInterface& scriptInterface) = 0;
 	virtual JS::Value GuiPollHistoricMessages(const ScriptInterface& scriptInterface) = 0;
 	virtual void SendMUCMessage(const std::string& message) = 0;
-	virtual void SendStunEndpointToHost(StunClient::StunEndpoint* stunEndpoint, const std::string& hostJID) = 0;
+	virtual void SendStunEndpointToHost(const StunClient::StunEndpoint& stunEndpoint, const std::string& hostJID) = 0;
 };
 
 extern IXmppClient *g_XmppClient;

source/lobby/XmppClient.cpp

@@ -1217,29 +1217,27 @@ std::string XmppClient::RegistrationResultToString(gloox::RegistrationResult res
 #undef CASE
 }
 
-void XmppClient::SendStunEndpointToHost(StunClient::StunEndpoint* stunEndpoint, const std::string& hostJIDStr)
+void XmppClient::SendStunEndpointToHost(const StunClient::StunEndpoint& stunEndpoint, const std::string& hostJIDStr)
 {
-	ENSURE(stunEndpoint);
-
 	char ipStr[256] = "(error)";
 	ENetAddress addr;
-	addr.host = ntohl(stunEndpoint->ip);
+	addr.host = ntohl(stunEndpoint.ip);
 	enet_address_get_host_ip(&addr, ipStr, ARRAY_SIZE(ipStr));
 
 	glooxwrapper::JID hostJID(hostJIDStr);
 	glooxwrapper::Jingle::Session session = m_sessionManager->createSession(hostJID);
-	session.sessionInitiate(ipStr, stunEndpoint->port);
+	session.sessionInitiate(ipStr, stunEndpoint.port);
 }
 
-void XmppClient::handleSessionAction(gloox::Jingle::Action action, glooxwrapper::Jingle::Session* UNUSED(session), const glooxwrapper::Jingle::Session::Jingle* jingle)
+void XmppClient::handleSessionAction(gloox::Jingle::Action action, glooxwrapper::Jingle::Session& session, const glooxwrapper::Jingle::Session::Jingle& jingle)
 {
 	if (action == gloox::Jingle::SessionInitiate)
-		handleSessionInitiation(jingle);
+		handleSessionInitiation(session, jingle);
 }
 
-void XmppClient::handleSessionInitiation(const glooxwrapper::Jingle::Session::Jingle* jingle)
+void XmppClient::handleSessionInitiation(glooxwrapper::Jingle::Session& UNUSED(session), const glooxwrapper::Jingle::Session::Jingle& jingle)
 {
-	glooxwrapper::Jingle::ICEUDP::Candidate candidate = jingle->getCandidate();
+	glooxwrapper::Jingle::ICEUDP::Candidate candidate = jingle.getCandidate();
 
 	if (candidate.ip.empty())
 	{

source/lobby/XmppClient.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2018 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -89,7 +89,7 @@ public:
 	void GUIGetBoardList(const ScriptInterface& scriptInterface, JS::MutableHandleValue ret);
 	void GUIGetProfile(const ScriptInterface& scriptInterface, JS::MutableHandleValue ret);
 
-	void SendStunEndpointToHost(StunClient::StunEndpoint* stunEndpoint, const std::string& hostJID);
+	void SendStunEndpointToHost(const StunClient::StunEndpoint& stunEndpoint, const std::string& hostJID);
 
 protected:
 	/* Xmpp handlers */
@@ -127,8 +127,8 @@ protected:
 	virtual void handleMessage(const glooxwrapper::Message& msg, glooxwrapper::MessageSession* session);
 
 	/* Session Handler */
-	virtual void handleSessionAction(gloox::Jingle::Action action, glooxwrapper::Jingle::Session* UNUSED(session), const glooxwrapper::Jingle::Session::Jingle* jingle);
-	virtual void handleSessionInitiation(const glooxwrapper::Jingle::Session::Jingle* jingle);
+	virtual void handleSessionAction(gloox::Jingle::Action action, glooxwrapper::Jingle::Session& session, const glooxwrapper::Jingle::Session::Jingle& jingle);
+	virtual void handleSessionInitiation(glooxwrapper::Jingle::Session& session, const glooxwrapper::Jingle::Session::Jingle& jingle);
 
 	// Helpers
 	void GetPresenceString(const gloox::Presence::PresenceType p, std::string& presence) const;

source/lobby/glooxwrapper/glooxwrapper.cpp

@@ -282,11 +282,17 @@ public:
 	SessionHandlerWrapper(glooxwrapper::Jingle::SessionHandler* wrapped, bool owned)
 		: m_Wrapped(wrapped), m_Owned(owned) {}
 
+	~SessionHandlerWrapper()
+	{
+		if (m_Owned)
+			delete m_Wrapped;
+	}
+
 	virtual void handleSessionAction(gloox::Jingle::Action action, gloox::Jingle::Session* session, const gloox::Jingle::Session::Jingle* jingle)
 	{
-		glooxwrapper::Jingle::Session wrapped_session(session, false);
-		glooxwrapper::Jingle::Session::Jingle wrapped_jingle(jingle, false);
-		m_Wrapped->handleSessionAction(action, &wrapped_session, &wrapped_jingle);
+		glooxwrapper::Jingle::Session sessionWrapper(session, false);
+		glooxwrapper::Jingle::Session::Jingle jingleWrapper(jingle, false);
+		m_Wrapped->handleSessionAction(action, sessionWrapper, jingleWrapper);
 	}
 
 	virtual void handleSessionActionError(gloox::Jingle::Action UNUSED(action), gloox::Jingle::Session* UNUSED(session), const gloox::Error* UNUSED(error))
@@ -376,8 +382,8 @@ void glooxwrapper::Client::disconnect()
 
 void glooxwrapper::Client::registerStanzaExtension(glooxwrapper::StanzaExtension* ext)
 {
-	gloox::StanzaExtension* stanza = new StanzaExtensionWrapper(ext, true);
-	m_Wrapped->registerStanzaExtension(stanza);
+	// ~StanzaExtensionFactory() deletes this new StanzaExtensionWrapper
+	m_Wrapped->registerStanzaExtension(new StanzaExtensionWrapper(ext, true));
 }
 
 void glooxwrapper::Client::registerConnectionListener(glooxwrapper::ConnectionListener* hnd)
@@ -790,24 +796,6 @@ const glooxwrapper::Jingle::Plugin glooxwrapper::Jingle::Plugin::findPlugin(int
 	return glooxwrapper::Jingle::Plugin(m_Wrapped->findPlugin(type), false);
 }
 
-glooxwrapper::Jingle::Content::Content(const string& name, const PluginList& plugins)
-	: glooxwrapper::Jingle::Plugin(NULL, false)
-{
-	gloox::Jingle::PluginList glooxPluginList;
-	for (const glooxwrapper::Jingle::Plugin* const& plugin: plugins)
-		glooxPluginList.push_back(plugin->getWrapped());
-
-	m_Wrapped = new gloox::Jingle::Content(name.to_string(), glooxPluginList);
-	m_Owned = true;
-}
-
-glooxwrapper::Jingle::Content::Content()
-	: glooxwrapper::Jingle::Plugin(NULL, false)
-{
-	m_Wrapped = new gloox::Jingle::Content();
-	m_Owned = true;
-}
-
 glooxwrapper::Jingle::ICEUDP::Candidate glooxwrapper::Jingle::Session::Jingle::getCandidate() const
 {
 	const gloox::Jingle::Content* content = static_cast<const gloox::Jingle::Content*>(m_Wrapped->plugins().front());
@@ -822,6 +810,17 @@ glooxwrapper::Jingle::ICEUDP::Candidate glooxwrapper::Jingle::Session::Jingle::g
 	return glooxwrapper::Jingle::ICEUDP::Candidate{glooxCandidate.ip, glooxCandidate.port};
 }
 
+glooxwrapper::Jingle::Session::Session(gloox::Jingle::Session* wrapped, bool owned)
+	: m_Wrapped(wrapped), m_Owned(owned)
+{
+}
+
+glooxwrapper::Jingle::Session::~Session()
+{
+	if (m_Owned)
+		delete m_Wrapped;
+}
+
 bool glooxwrapper::Jingle::Session::sessionInitiate(char* ipStr, u16 port)
 {
 	gloox::Jingle::ICEUDP::CandidateList candidateList;
@@ -842,52 +841,15 @@ bool glooxwrapper::Jingle::Session::sessionInitiate(char* ipStr, u16 port)
 		gloox::Jingle::ICEUDP::ServerReflexive
 	});
 
+	// sessionInitiate deletes the new Content, and
+	// the Plugin destructor inherited by Content frees the ICEUDP plugin.
+
 	gloox::Jingle::PluginList pluginList;
 	pluginList.push_back(new gloox::Jingle::ICEUDP(/*local_pwd*/"", /*local_ufrag*/"", candidateList));
-	// This is safe as gloox will free Content which will free the ICEUDP object
+
 	return m_Wrapped->sessionInitiate(new gloox::Jingle::Content(std::string("game-data"), pluginList));
 }
 
-glooxwrapper::Jingle::ICEUDP::ICEUDP(glooxwrapper::Jingle::ICEUDP::CandidateList& candidates)
-	: glooxwrapper::Jingle::Plugin(NULL, false)
-{
-	gloox::Jingle::ICEUDP::CandidateList glooxCandidates;
-	for (const glooxwrapper::Jingle::ICEUDP::Candidate& candidate : candidates)
-		glooxCandidates.push_back(gloox::Jingle::ICEUDP::Candidate
-			{
-				"1", // component_id,
-				"1", // foundation
-				"0", // candidate_generation
-				"1", // candidate_id
-				candidate.ip.to_string(),
-				"0", // network
-				candidate.port,
-				0, // priority
-				"udp",
-				"", // base_ip
-				0, // base_port
-				gloox::Jingle::ICEUDP::ServerReflexive
-			});
-
-	m_Wrapped = new gloox::Jingle::ICEUDP(/*local_pwd*/"", /*local_ufrag*/"", glooxCandidates);
-	m_Owned = true;
-}
-
-glooxwrapper::Jingle::ICEUDP::ICEUDP()
-	: glooxwrapper::Jingle::Plugin(NULL, false)
-{
-	m_Wrapped = new gloox::Jingle::ICEUDP();
-	m_Owned = true;
-}
-
-const glooxwrapper::Jingle::ICEUDP::CandidateList glooxwrapper::Jingle::ICEUDP::candidates() const
-{
-	glooxwrapper::Jingle::ICEUDP::CandidateList candidateListWrapper;
-	for (const gloox::Jingle::ICEUDP::Candidate& candidate : static_cast<const gloox::Jingle::ICEUDP*>(m_Wrapped)->candidates())
-		candidateListWrapper.push_back(glooxwrapper::Jingle::ICEUDP::Candidate{candidate.ip, candidate.port});
-	return candidateListWrapper;
-}
-
 glooxwrapper::SessionManager::SessionManager(Client* parent, Jingle::SessionHandler* sh)
 {
 	m_HandlerWrapper = new SessionHandlerWrapper(sh, false);
@@ -902,12 +864,17 @@ glooxwrapper::SessionManager::~SessionManager()
 
 void glooxwrapper::SessionManager::registerPlugins()
 {
+	// This calls m_factory.registerPlugin (see jinglesessionmanager.cpp), hence
+	// ~PluginFactory() will delete these new plugin templates.
 	m_Wrapped->registerPlugin(new gloox::Jingle::Content());
 	m_Wrapped->registerPlugin(new gloox::Jingle::ICEUDP());
 }
 
 glooxwrapper::Jingle::Session glooxwrapper::SessionManager::createSession(const JID& callee)
 {
+	// The wrapped gloox SessionManager keeps track of this session and deletes it on ~SessionManager().
 	gloox::Jingle::Session* glooxSession = m_Wrapped->createSession(callee.getWrapped(), m_HandlerWrapper);
+
+	// Hence the glooxwrapper::Jingle::Session may not own the gloox::Jingle::Session.
 	return glooxwrapper::Jingle::Session(glooxSession, false);
 }

source/lobby/glooxwrapper/glooxwrapper.h

@@ -585,6 +585,9 @@ namespace glooxwrapper
 		ConstTagList findTagList_clone(const string& expression) const; // like findTagList but each tag must be Tag::free()d
 	};
 
+	/**
+	 * See XEP-0166: Jingle and https://camaya.net/api/gloox/namespacegloox_1_1Jingle.html.
+	 */
 	namespace Jingle
 	{
 
@@ -605,27 +608,19 @@ namespace glooxwrapper
 
 		typedef list<const Plugin*> PluginList;
 
-		class GLOOXWRAPPER_API Content : public Plugin
-		{
-		public:
-			Content(const string& name, const PluginList& plugins);
-			Content();
-		};
-
-		class GLOOXWRAPPER_API ICEUDP : public Plugin
+		/**
+		 * See XEP-0176: Jingle ICE-UDP Transport Method
+		 */
+		class GLOOXWRAPPER_API ICEUDP
 		{
 		public:
 			struct Candidate {
 				string ip;
 				int port;
 			};
-
-			typedef std::list<Candidate> CandidateList;
-
-			ICEUDP(CandidateList& candidates);
-			ICEUDP();
-
-			const CandidateList candidates() const;
+		private:
+			// Class not implemented as it is not used.
+			ICEUDP() = delete;
 		};
 
 		class GLOOXWRAPPER_API Session
@@ -646,7 +641,8 @@ namespace glooxwrapper
 				ICEUDP::Candidate getCandidate() const;
 			};
 
-			Session(gloox::Jingle::Session* wrapped, bool owned) : m_Wrapped(wrapped), m_Owned(owned) {}
+			Session(gloox::Jingle::Session* wrapped, bool owned);
+			~Session();
 
 			bool sessionInitiate(char* ipStr, uint16_t port);
 		};
@@ -655,7 +651,7 @@ namespace glooxwrapper
 		{
 		public:
 			virtual ~SessionHandler() {}
-			virtual void handleSessionAction(gloox::Jingle::Action action, Session* session, const Session::Jingle* jingle) = 0;
+			virtual void handleSessionAction(gloox::Jingle::Action action, Session& session, const Session::Jingle& jingle) = 0;
 		};
 
 	}

source/network/NetClient.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2018 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -107,7 +107,7 @@ public:
 	 * @param server IP address or host name to connect to
 	 * @return true on success, false on connection failure
 	 */
-	bool SetupConnection(const CStr& server, const u16 port, ENetHost* enetClient = NULL);
+	bool SetupConnection(const CStr& server, const u16 port, ENetHost* enetClient);
 
 	/**
 	 * Destroy the connection to the server.

source/network/NetServer.cpp

@@ -1574,7 +1574,8 @@ CStrW CNetServerWorker::DeduplicatePlayerName(const CStrW& original)
 
 void CNetServerWorker::SendHolePunchingMessage(const CStr& ipStr, u16 port)
 {
-	StunClient::SendHolePunchingMessages(m_Host, ipStr.c_str(), port);
+	if (m_Host)
+		StunClient::SendHolePunchingMessages(*m_Host, ipStr, port);
 }
 
 

source/network/StunClient.cpp

@@ -129,10 +129,8 @@ bool GetFromBuffer(const std::vector<u8>& buffer, u32& offset, T& result)
  * The request is sent through transactionHost, from which the answer
  * will be retrieved by ReceiveStunResponse and interpreted by ParseStunResponse.
  */
-bool CreateStunRequest(ENetHost* transactionHost)
+bool CreateStunRequest(ENetHost& transactionHost)
 {
-	ENSURE(transactionHost);
-
 	CStr server_name;
 	CFG_GET_VAL("lobby.stun.server", server_name);
 	CFG_GET_VAL("lobby.stun.port", m_StunServerPort);
@@ -161,7 +159,7 @@ bool CreateStunRequest(ENetHost* transactionHost)
 	ENSURE(res);
 
 	// Documentation says it points to "one or more addrinfo structures"
-	sockaddr_in* current_interface = (sockaddr_in*)(res->ai_addr);
+	sockaddr_in* current_interface = reinterpret_cast<sockaddr_in*>(res->ai_addr);
 	m_StunServerIP = ntohl(current_interface->sin_addr.s_addr);
 
 	StunClient::SendStunRequest(transactionHost, m_StunServerIP, m_StunServerPort);
@@ -170,7 +168,7 @@ bool CreateStunRequest(ENetHost* transactionHost)
 	return true;
 }
 
-void StunClient::SendStunRequest(ENetHost* transactionHost, u32 targetIp, u16 targetPort)
+void StunClient::SendStunRequest(ENetHost& transactionHost, u32 targetIp, u16 targetPort)
 {
 	std::vector<u8> buffer;
 	AddUInt16(buffer, m_MethodTypeBinding);
@@ -192,16 +190,20 @@ void StunClient::SendStunRequest(ENetHost* transactionHost, u32 targetIp, u16 ta
 	to.sin_port = htons(targetPort);
 	to.sin_addr.s_addr = htonl(targetIp);
 
-	sendto(transactionHost->socket, (char*)(buffer.data()), (int)buffer.size(), 0, (sockaddr*)&to, to_len);
+	sendto(
+		transactionHost.socket,
+		reinterpret_cast<char*>(buffer.data()),
+		static_cast<int>(buffer.size()),
+		0,
+		reinterpret_cast<sockaddr*>(&to),
+		to_len);
 }
 
 /**
  * Gets the response from the STUN server and checks it for its validity.
  */
-bool ReceiveStunResponse(ENetHost* transactionHost, std::vector<u8>& buffer)
+bool ReceiveStunResponse(ENetHost& transactionHost, std::vector<u8>& buffer)
 {
-	ENSURE(transactionHost);
-
 	// TransportAddress sender;
 	const int LEN = 2048;
 	char input_buffer[LEN];
@@ -211,7 +213,7 @@ bool ReceiveStunResponse(ENetHost* transactionHost, std::vector<u8>& buffer)
 	sockaddr_in addr;
 	socklen_t from_len = sizeof(addr);
 
-	int len = recvfrom(transactionHost->socket, input_buffer, LEN, 0, (sockaddr*)(&addr), &from_len);
+	int len = recvfrom(transactionHost.socket, input_buffer, LEN, 0, reinterpret_cast<sockaddr*>(&addr), &from_len);
 
 	int delay = 200;
 	CFG_GET_VAL("lobby.stun.delay", delay);
@@ -221,7 +223,7 @@ bool ReceiveStunResponse(ENetHost* transactionHost, std::vector<u8>& buffer)
 	for (int count = 0; len < 0 && (count < max_tries || max_tries == -1); ++count)
 	{
 		usleep(delay * 1000);
-		len = recvfrom(transactionHost->socket, input_buffer, LEN, 0, (sockaddr*)(&addr), &from_len);
+		len = recvfrom(transactionHost.socket, input_buffer, LEN, 0, reinterpret_cast<sockaddr*>(&addr), &from_len);
 	}
 
 	if (len < 0)
@@ -230,7 +232,7 @@ bool ReceiveStunResponse(ENetHost* transactionHost, std::vector<u8>& buffer)
 		return false;
 	}
 
-	u32 sender_ip = ntohl((u32)(addr.sin_addr.s_addr));
+	u32 sender_ip = ntohl(static_cast<u32>(addr.sin_addr.s_addr));
 	u16 sender_port = ntohs(addr.sin_port);
 
 	if (sender_ip != m_StunServerIP)
@@ -246,7 +248,7 @@ bool ReceiveStunResponse(ENetHost* transactionHost, std::vector<u8>& buffer)
 
 	// Convert to network string.
 	buffer.resize(len);
-	memcpy(buffer.data(), (u8*)input_buffer, len);
+	memcpy(buffer.data(), reinterpret_cast<u8*>(input_buffer), len);
 
 	return true;
 }
@@ -357,7 +359,7 @@ bool ParseStunResponse(const std::vector<u8>& buffer)
 	return true;
 }
 
-bool STUNRequestAndResponse(ENetHost* transactionHost)
+bool STUNRequestAndResponse(ENetHost& transactionHost)
 {
 	if (!CreateStunRequest(transactionHost))
 		return false;
@@ -369,7 +371,7 @@ bool STUNRequestAndResponse(ENetHost* transactionHost)
 
 JS::Value StunClient::FindStunEndpointHost(const ScriptInterface& scriptInterface, int port)
 {
-	ENetAddress hostAddr{ENET_HOST_ANY, (u16)port};
+	ENetAddress hostAddr{ENET_HOST_ANY, static_cast<u16>(port)};
 	ENetHost* transactionHost = enet_host_create(&hostAddr, 1, 1, 0, 0);
 	if (!transactionHost)
 	{
@@ -377,7 +379,7 @@ JS::Value StunClient::FindStunEndpointHost(const ScriptInterface& scriptInterfac
 		return JS::UndefinedValue();
 	}
 
-	bool success = STUNRequestAndResponse(transactionHost);
+	bool success = STUNRequestAndResponse(*transactionHost);
 	enet_host_destroy(transactionHost);
 	if (!success)
 		return JS::UndefinedValue();
@@ -396,12 +398,10 @@ JS::Value StunClient::FindStunEndpointHost(const ScriptInterface& scriptInterfac
 	return stunEndpoint;
 }
 
-StunClient::StunEndpoint* StunClient::FindStunEndpointJoin(ENetHost* transactionHost)
+bool StunClient::FindStunEndpointJoin(ENetHost& transactionHost, StunClient::StunEndpoint& stunEndpoint)
 {
-	ENSURE(transactionHost);
-
 	if (!STUNRequestAndResponse(transactionHost))
-		return nullptr;
+		return false;
 
 	// Convert m_IP to string
 	char ipStr[256] = "(error)";
@@ -409,15 +409,18 @@ StunClient::StunEndpoint* StunClient::FindStunEndpointJoin(ENetHost* transaction
 	addr.host = ntohl(m_IP);
 	enet_address_get_host_ip(&addr, ipStr, ARRAY_SIZE(ipStr));
 
-	return new StunEndpoint({ m_IP, m_Port });
+	stunEndpoint.ip = m_IP;
+	stunEndpoint.port = m_Port;
+
+	return true;
 }
 
-void StunClient::SendHolePunchingMessages(ENetHost* enetClient, const char* serverAddress, u16 serverPort)
+void StunClient::SendHolePunchingMessages(ENetHost& enetClient, const std::string& serverAddress, u16 serverPort)
 {
 	// Convert ip string to int64
 	ENetAddress addr;
 	addr.port = serverPort;
-	enet_address_set_host(&addr, serverAddress);
+	enet_address_set_host(&addr, serverAddress.c_str());
 
 	int delay = 200;
 	CFG_GET_VAL("lobby.stun.delay", delay);

source/network/StunClient.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2017 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  * Copyright (C) 2013-2016 SuperTuxKart-Team.
  * This file is part of 0 A.D.
  *
@@ -21,6 +21,8 @@
 
 #include "scriptinterface/ScriptInterface.h"
 
+#include <string>
+
 typedef struct _ENetHost ENetHost;
 
 namespace StunClient
@@ -31,13 +33,13 @@ struct StunEndpoint {
 	u16 port;
 };
 
-void SendStunRequest(ENetHost* transactionHost, u32 targetIp, u16 targetPort);
+void SendStunRequest(ENetHost& transactionHost, u32 targetIp, u16 targetPort);
 
 JS::Value FindStunEndpointHost(const ScriptInterface& scriptInterface, int port);
 
-StunEndpoint* FindStunEndpointJoin(ENetHost* transactionHost);
+bool FindStunEndpointJoin(ENetHost& transactionHost, StunClient::StunEndpoint& stunEndpoint);
 
-void SendHolePunchingMessages(ENetHost* enetClient, const char* serverAddress, u16 serverPort);
+void SendHolePunchingMessages(ENetHost& enetClient, const std::string& serverAddress, u16 serverPort);
 
 }
 

source/network/scripting/JSInterface_Network.cpp

@@ -1,4 +1,4 @@
-/* Copyright (C) 2018 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -71,7 +71,7 @@ void JSI_Network::StartNetworkHost(ScriptInterface::CxPrivate* pCxPrivate, const
 	g_NetClient->SetUserName(playerName);
 	g_NetClient->SetHostingPlayerName(hostLobbyName);
 
-	if (!g_NetClient->SetupConnection("127.0.0.1", serverPort))
+	if (!g_NetClient->SetupConnection("127.0.0.1", serverPort, nullptr))
 	{
 		pCxPrivate->pScriptInterface->ReportError("Failed to connect to server");
 		SAFE_DELETE(g_NetClient);
@@ -104,15 +104,14 @@ void JSI_Network::StartNetworkJoin(ScriptInterface::CxPrivate* pCxPrivate, const
 			return;
 		}
 
-		StunClient::StunEndpoint* stunEndpoint = StunClient::FindStunEndpointJoin(enetClient);
-		if (!stunEndpoint)
+		StunClient::StunEndpoint stunEndpoint;
+		if (!StunClient::FindStunEndpointJoin(*enetClient, stunEndpoint))
 		{
 			pCxPrivate->pScriptInterface->ReportError("Could not find the STUN endpoint");
 			return;
 		}
 
 		g_XmppClient->SendStunEndpointToHost(stunEndpoint, hostJID);
-		delete stunEndpoint;
 
 		SDL_Delay(1000);
 	}
@@ -123,7 +122,7 @@ void JSI_Network::StartNetworkJoin(ScriptInterface::CxPrivate* pCxPrivate, const
 	g_NetClient->SetHostingPlayerName(hostJID.substr(0, hostJID.find("@")));
 
 	if (g_XmppClient && useSTUN)
-		StunClient::SendHolePunchingMessages(enetClient, serverAddress.c_str(), serverPort);
+		StunClient::SendHolePunchingMessages(*enetClient, serverAddress, serverPort);
 
 	if (!g_NetClient->SetupConnection(serverAddress, serverPort, enetClient))
 	{

source/network/tests/test_Net.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2017 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -75,7 +75,7 @@ public:
 	{
 		TS_ASSERT(server.SetupConnection(PS_DEFAULT_PORT));
 		for (size_t j = 0; j < clients.size(); ++j)
-			TS_ASSERT(clients[j]->SetupConnection("127.0.0.1", PS_DEFAULT_PORT));
+			TS_ASSERT(clients[j]->SetupConnection("127.0.0.1", PS_DEFAULT_PORT, nullptr));
 
 		for (size_t i = 0; ; ++i)
 		{
@@ -275,7 +275,7 @@ public:
 		client2B.SetUserName(L"bob");
 		clients.push_back(&client2B);
 
-		TS_ASSERT(client2B.SetupConnection("127.0.0.1", PS_DEFAULT_PORT));
+		TS_ASSERT(client2B.SetupConnection("127.0.0.1", PS_DEFAULT_PORT, nullptr));
 
 		for (size_t i = 0; ; ++i)
 		{

source/ps/GameSetup/GameSetup.cpp

@@ -1605,7 +1605,7 @@ bool Autostart(const CmdLineArgs& args)
 
 		g_NetClient = new CNetClient(g_Game, true);
 		g_NetClient->SetUserName(userName);
-		g_NetClient->SetupConnection("127.0.0.1", PS_DEFAULT_PORT);
+		g_NetClient->SetupConnection("127.0.0.1", PS_DEFAULT_PORT, nullptr);
 	}
 	else if (args.Has("autostart-client"))
 	{
@@ -1618,7 +1618,7 @@ bool Autostart(const CmdLineArgs& args)
 		if (ip.empty())
 			ip = "127.0.0.1";
 
-		bool ok = g_NetClient->SetupConnection(ip, PS_DEFAULT_PORT);
+		bool ok = g_NetClient->SetupConnection(ip, PS_DEFAULT_PORT, nullptr);
 		ENSURE(ok);
 	}
 	else