Skip to content

Commit 767bfec

Browse files
committed
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.
1 parent 79de2ea commit 767bfec

File tree

2 files changed

+10
-23
lines changed

2 files changed

+10
-23
lines changed

modules/upnp/doc_classes/UPNPDevice.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
<constant name="IGD_STATUS_HTTP_EMPTY" value="2" enum="IGDStatus">
7272
Empty HTTP response.
7373
</constant>
74-
<constant name="IGD_STATUS_NO_URLS" value="3" enum="IGDStatus">
74+
<constant name="IGD_STATUS_NO_URLS" value="3" enum="IGDStatus" deprecated="This value is no longer used.">
7575
Returned response contained no URLs.
7676
</constant>
7777
<constant name="IGD_STATUS_NO_IGD" value="4" enum="IGDStatus">
@@ -86,7 +86,7 @@
8686
<constant name="IGD_STATUS_INVALID_CONTROL" value="7" enum="IGDStatus">
8787
Invalid control.
8888
</constant>
89-
<constant name="IGD_STATUS_MALLOC_ERROR" value="8" enum="IGDStatus">
89+
<constant name="IGD_STATUS_MALLOC_ERROR" value="8" enum="IGDStatus" deprecated="This value is no longer used.">
9090
Memory allocation error.
9191
</constant>
9292
<constant name="IGD_STATUS_UNKNOWN_ERROR" value="9" enum="IGDStatus">

modules/upnp/upnp.cpp

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -121,33 +121,20 @@ void UPNP::parse_igd(Ref<UPNPDevice> dev, UPNPDev *devlist) {
121121
return;
122122
}
123123

124-
struct UPNPUrls *urls = (UPNPUrls *)malloc(sizeof(struct UPNPUrls));
125-
126-
if (!urls) {
127-
dev->set_igd_status(UPNPDevice::IGD_STATUS_MALLOC_ERROR);
128-
return;
129-
}
130-
124+
struct UPNPUrls urls = {};
131125
struct IGDdatas data;
132126

133-
memset(urls, 0, sizeof(struct UPNPUrls));
134-
135127
parserootdesc(xml, size, &data);
136128
free(xml);
137129
xml = nullptr;
138130

139-
GetUPNPUrls(urls, &data, dev->get_description_url().utf8().get_data(), 0);
140-
141-
if (!urls) {
142-
dev->set_igd_status(UPNPDevice::IGD_STATUS_NO_URLS);
143-
return;
144-
}
131+
GetUPNPUrls(&urls, &data, dev->get_description_url().utf8().get_data(), 0);
145132

146133
char addr[16];
147-
int i = UPNP_GetValidIGD(devlist, urls, &data, (char *)&addr, 16);
134+
int i = UPNP_GetValidIGD(devlist, &urls, &data, (char *)&addr, 16);
148135

149136
if (i != 1) {
150-
FreeUPNPUrls(urls);
137+
FreeUPNPUrls(&urls);
151138

152139
switch (i) {
153140
case 0:
@@ -165,18 +152,18 @@ void UPNP::parse_igd(Ref<UPNPDevice> dev, UPNPDev *devlist) {
165152
}
166153
}
167154

168-
if (urls->controlURL[0] == '\0') {
169-
FreeUPNPUrls(urls);
155+
if (urls.controlURL[0] == '\0') {
156+
FreeUPNPUrls(&urls);
170157
dev->set_igd_status(UPNPDevice::IGD_STATUS_INVALID_CONTROL);
171158
return;
172159
}
173160

174-
dev->set_igd_control_url(urls->controlURL);
161+
dev->set_igd_control_url(urls.controlURL);
175162
dev->set_igd_service_type(data.first.servicetype);
176163
dev->set_igd_our_addr(addr);
177164
dev->set_igd_status(UPNPDevice::IGD_STATUS_OK);
178165

179-
FreeUPNPUrls(urls);
166+
FreeUPNPUrls(&urls);
180167
}
181168

182169
int UPNP::upnp_result(int in) {

0 commit comments

Comments
 (0)