Browse Source

Merge pull request #89897 from alesliehughes/upnp_memory_leak

UPNP: Use local variable for UPNPUrls to stop memory leak.
Rémi Verschelde 1 year ago
parent
commit
d619ffdb75
2 changed files with 10 additions and 23 deletions
  1. 2 2
      modules/upnp/doc_classes/UPNPDevice.xml
  2. 8 21
      modules/upnp/upnp.cpp

+ 2 - 2
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">

+ 8 - 21
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) {