From 767bfec8b63bf88151f27e2fa00f5bd363be6091 Mon Sep 17 00:00:00 2001
From: Alistair Leslie-Hughes <leslie_alistair@hotmail.com>
Date: Tue, 26 Mar 2024 13:13:43 +1100
Subject: [PATCH] Use local variable to stop memory leak.

I've change the urls variable to be a local, instead of manually allocating it all the time.
This is only used locally and was causing a memory leak because FreeUPNPUrls gave the impression it free it.

1. FreeUPNPUrls doesn't free the pointer passed in, that's up to caller.
2. The second if(!urls) produced dead code as we checked the pointer just after allocation.
---
 modules/upnp/doc_classes/UPNPDevice.xml |  4 ++--
 modules/upnp/upnp.cpp                   | 29 +++++++------------------
 2 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/modules/upnp/doc_classes/UPNPDevice.xml b/modules/upnp/doc_classes/UPNPDevice.xml
index a70ae1b9cc0f..82ac179611e4 100644
--- a/modules/upnp/doc_classes/UPNPDevice.xml
+++ b/modules/upnp/doc_classes/UPNPDevice.xml
@@ -71,7 +71,7 @@
 		<constant name="IGD_STATUS_HTTP_EMPTY" value="2" enum="IGDStatus">
 			Empty HTTP response.
 		</constant>
-		<constant name="IGD_STATUS_NO_URLS" value="3" enum="IGDStatus">
+		<constant name="IGD_STATUS_NO_URLS" value="3" enum="IGDStatus" deprecated="This value is no longer used.">
 			Returned response contained no URLs.
 		</constant>
 		<constant name="IGD_STATUS_NO_IGD" value="4" enum="IGDStatus">
@@ -86,7 +86,7 @@
 		<constant name="IGD_STATUS_INVALID_CONTROL" value="7" enum="IGDStatus">
 			Invalid control.
 		</constant>
-		<constant name="IGD_STATUS_MALLOC_ERROR" value="8" enum="IGDStatus">
+		<constant name="IGD_STATUS_MALLOC_ERROR" value="8" enum="IGDStatus" deprecated="This value is no longer used.">
 			Memory allocation error.
 		</constant>
 		<constant name="IGD_STATUS_UNKNOWN_ERROR" value="9" enum="IGDStatus">
diff --git a/modules/upnp/upnp.cpp b/modules/upnp/upnp.cpp
index 2812f37eb23c..95453c1ecdad 100644
--- a/modules/upnp/upnp.cpp
+++ b/modules/upnp/upnp.cpp
@@ -121,33 +121,20 @@ void UPNP::parse_igd(Ref<UPNPDevice> dev, UPNPDev *devlist) {
 		return;
 	}
 
-	struct UPNPUrls *urls = (UPNPUrls *)malloc(sizeof(struct UPNPUrls));
-
-	if (!urls) {
-		dev->set_igd_status(UPNPDevice::IGD_STATUS_MALLOC_ERROR);
-		return;
-	}
-
+	struct UPNPUrls urls = {};
 	struct IGDdatas data;
 
-	memset(urls, 0, sizeof(struct UPNPUrls));
-
 	parserootdesc(xml, size, &data);
 	free(xml);
 	xml = nullptr;
 
-	GetUPNPUrls(urls, &data, dev->get_description_url().utf8().get_data(), 0);
-
-	if (!urls) {
-		dev->set_igd_status(UPNPDevice::IGD_STATUS_NO_URLS);
-		return;
-	}
+	GetUPNPUrls(&urls, &data, dev->get_description_url().utf8().get_data(), 0);
 
 	char addr[16];
-	int i = UPNP_GetValidIGD(devlist, urls, &data, (char *)&addr, 16);
+	int i = UPNP_GetValidIGD(devlist, &urls, &data, (char *)&addr, 16);
 
 	if (i != 1) {
-		FreeUPNPUrls(urls);
+		FreeUPNPUrls(&urls);
 
 		switch (i) {
 			case 0:
@@ -165,18 +152,18 @@ void UPNP::parse_igd(Ref<UPNPDevice> dev, UPNPDev *devlist) {
 		}
 	}
 
-	if (urls->controlURL[0] == '\0') {
-		FreeUPNPUrls(urls);
+	if (urls.controlURL[0] == '\0') {
+		FreeUPNPUrls(&urls);
 		dev->set_igd_status(UPNPDevice::IGD_STATUS_INVALID_CONTROL);
 		return;
 	}
 
-	dev->set_igd_control_url(urls->controlURL);
+	dev->set_igd_control_url(urls.controlURL);
 	dev->set_igd_service_type(data.first.servicetype);
 	dev->set_igd_our_addr(addr);
 	dev->set_igd_status(UPNPDevice::IGD_STATUS_OK);
 
-	FreeUPNPUrls(urls);
+	FreeUPNPUrls(&urls);
 }
 
 int UPNP::upnp_result(int in) {