Browse code

lost: fixing a memory leak and minor code refactoring and improvements

Wolfgang Kampichler authored on 18/04/2021 16:08:13
Showing 3 changed files
... ...
@@ -154,6 +154,7 @@ err:
154 154
 	/* clean up */
155 155
 	if(ret != NULL) {
156 156
 		pkg_free(ret);
157
+		ret = NULL;
157 158
 	}
158 159
 	*lgth = 0;
159 160
 	return NULL;
... ...
@@ -302,7 +303,7 @@ int lost_held_function(struct sip_msg *_m, char *_con, char *_pidf, char *_url,
302 303
 			/* is it a name or ip ... check nameinfo (reverse lookup) */
303 304
 			len = 0;
304 305
 			ipstr = lost_copy_string(host, &len);
305
-			if(len > 0) {
306
+			if(ipstr != NULL && len > 0) {
306 307
 				name.s = &(istr[0]);
307 308
 				name.len = NI_MAXHOST;
308 309
 				if(lost_get_nameinfo(ipstr, &name, flag) > 0) {
... ...
@@ -318,6 +319,7 @@ int lost_held_function(struct sip_msg *_m, char *_con, char *_pidf, char *_url,
318 319
 					LM_DBG("no nameinfo for [%s]\n", ipstr);
319 320
 				}
320 321
 				pkg_free(ipstr); /* clean up */
322
+				ipstr = NULL;
321 323
 			}
322 324
 			url.s = &(ustr[0]);
323 325
 			url.len = MAX_URI_SIZE;
... ...
@@ -345,7 +347,9 @@ int lost_held_function(struct sip_msg *_m, char *_con, char *_pidf, char *_url,
345 347
 			curl = httpapi.http_client_query_c(
346 348
 					_m, lisurl, &res, que.s, mtheld, ACCEPT_HDR);
347 349
 			pkg_free(lisurl); /*clean up */
350
+			lisurl = NULL;
348 351
 		} else {
352
+			LM_ERR("could not copy POST url\n");
349 353
 			goto err;
350 354
 		}
351 355
 	}
... ...
@@ -356,7 +360,6 @@ int lost_held_function(struct sip_msg *_m, char *_con, char *_pidf, char *_url,
356 360
 		} else {
357 361
 			LM_ERR("POST [%.*s] failed with error: %d\n", url.len, url.s, curl);
358 362
 		}
359
-		lost_free_string(&res);
360 363
 		goto err;
361 364
 	}
362 365
 	if(con.s != NULL && con.len > 0) {
... ...
@@ -440,6 +443,7 @@ int lost_held_function(struct sip_msg *_m, char *_con, char *_pidf, char *_url,
440 443
 					curl = httpapi.http_client_query_c(
441 444
 							_m, geo.s, &pidfurl, heldreq, mtheld, ACCEPT_HDR);
442 445
 					pkg_free(heldreq); /* clean up */
446
+					heldreq = NULL;
443 447
 				} else {
444 448
 					LM_ERR("could not create POST request\n");
445 449
 					lost_free_string(&pidfurl); /* clean up */
... ...
@@ -495,6 +499,7 @@ int lost_held_function(struct sip_msg *_m, char *_con, char *_pidf, char *_url,
495 499
 	pvpidf.flags = PV_VAL_STR;
496 500
 	pspidf = (pv_spec_t *)_pidf;
497 501
 	pspidf->setf(_m, &pspidf->pvp, (int)EQ_T, &pvpidf);
502
+	lost_free_string(&res); /* clean up */
498 503
 
499 504
 	pvurl.rs = geo;
500 505
 	pvurl.rs.s = geo.s;
... ...
@@ -503,7 +508,8 @@ int lost_held_function(struct sip_msg *_m, char *_con, char *_pidf, char *_url,
503 508
 	pvurl.flags = PV_VAL_STR;
504 509
 	psurl = (pv_spec_t *)_url;
505 510
 	psurl->setf(_m, &psurl->pvp, (int)EQ_T, &pvurl);
506
-
511
+	lost_free_string(&geo); /* clean up */
512
+ 
507 513
 	pverr.rs = err;
508 514
 	pverr.rs.s = err.s;
509 515
 	pverr.rs.len = err.len;
... ...
@@ -511,14 +517,25 @@ int lost_held_function(struct sip_msg *_m, char *_con, char *_pidf, char *_url,
511 517
 	pverr.flags = PV_VAL_STR;
512 518
 	pserr = (pv_spec_t *)_err;
513 519
 	pserr->setf(_m, &pserr->pvp, (int)EQ_T, &pverr);
520
+	lost_free_string(&err); /* clean up */
514 521
 
515 522
 	return (err.len > 0) ? LOST_SERVER_ERROR : LOST_SUCCESS;
516 523
 
517 524
 err:
525
+	/* clean up */
518 526
 	if(doc != NULL) {
519 527
 		xmlFreeDoc(doc);
520 528
 	}
521
-
529
+	if(res.s != NULL && res.len > 0) {
530
+		lost_free_string(&res);
531
+	}
532
+	if(geo.s != NULL && geo.len > 0) {
533
+		lost_free_string(&geo);
534
+	}
535
+	if(err.s != NULL && err.len > 0) {
536
+		lost_free_string(&err);
537
+	}
538
+	
522 539
 	return LOST_CLIENT_ERROR;
523 540
 }
524 541
 
... ...
@@ -629,6 +646,7 @@ int lost_held_dereference(struct sip_msg *_m, char *_url, char *_pidf,
629 646
 	/* clean up */
630 647
 	if(rtype != NULL) {
631 648
 		pkg_free(rtype);
649
+		rtype = NULL;
632 650
 	}
633 651
 
634 652
 	if(heldreq != NULL && len == 0) {
... ...
@@ -648,10 +666,13 @@ int lost_held_dereference(struct sip_msg *_m, char *_url, char *_pidf,
648 666
 		curl = httpapi.http_client_query_c(
649 667
 				_m, lisurl, &res, heldreq, mtheld, ACCEPT_HDR);
650 668
 		pkg_free(lisurl); /* clean up */
669
+		lisurl = NULL;
651 670
 		pkg_free(heldreq);
671
+		heldreq = NULL;
652 672
 	} else {
653 673
 		LM_ERR("could not copy POST url\n");
654 674
 		pkg_free(heldreq); /* clean up */
675
+		heldreq = NULL;
655 676
 		goto err;
656 677
 	}
657 678
 
... ...
@@ -722,6 +743,7 @@ int lost_held_dereference(struct sip_msg *_m, char *_url, char *_pidf,
722 743
 	pvpidf.flags = PV_VAL_STR;
723 744
 	pspidf = (pv_spec_t *)_pidf;
724 745
 	pspidf->setf(_m, &pspidf->pvp, (int)EQ_T, &pvpidf);
746
+	lost_free_string(&res); /* clean up */
725 747
 
726 748
 	pverr.rs = err;
727 749
 	pverr.rs.s = err.s;
... ...
@@ -730,16 +752,21 @@ int lost_held_dereference(struct sip_msg *_m, char *_url, char *_pidf,
730 752
 	pverr.flags = PV_VAL_STR;
731 753
 	pserr = (pv_spec_t *)_err;
732 754
 	pserr->setf(_m, &pserr->pvp, (int)EQ_T, &pverr);
755
+	lost_free_string(&err); /* clean up */
733 756
 
734 757
 	return (err.len > 0) ? LOST_SERVER_ERROR : LOST_SUCCESS;
735 758
 
736 759
 err:
760
+	/* clean up */
737 761
 	if(doc != NULL) {
738 762
 		xmlFreeDoc(doc);
739 763
 	}
740 764
 	if(res.s != NULL && res.len > 0) {
741 765
 		lost_free_string(&res);
742 766
 	}
767
+	if(err.s != NULL && err.len > 0) {
768
+		lost_free_string(&err);
769
+	}
743 770
 
744 771
 	return LOST_CLIENT_ERROR;
745 772
 }
... ...
@@ -863,7 +890,7 @@ int lost_function(struct sip_msg *_m, char *_con, char *_uri, char *_name,
863 890
 			}
864 891
 		}
865 892
 	}
866
-	/* neither valifd pidf parameter nor loc ... check geolocation header */
893
+	/* neither valid pidf parameter nor loc ... check geolocation header */
867 894
 	if(loc == NULL) {
868 895
 
869 896
 		/* parse Geolocation header */
... ...
@@ -969,6 +996,7 @@ int lost_function(struct sip_msg *_m, char *_con, char *_uri, char *_name,
969 996
 					curl = httpapi.http_client_query_c(
970 997
 							_m, url.s, &ret, heldreq, mtheld, ACCEPT_HDR);
971 998
 					pkg_free(heldreq); /* clean up */
999
+					heldreq = NULL;
972 1000
 				} else {
973 1001
 					LM_ERR("could not create POST request\n");
974 1002
 					goto err;
... ...
@@ -1002,13 +1030,15 @@ int lost_function(struct sip_msg *_m, char *_con, char *_uri, char *_name,
1002 1030
 		/* clean up */
1003 1031
 		lost_free_geoheader_list(&geolist);
1004 1032
 		lost_free_string(&ret);
1033
+
1034
+		if(pidf.s == NULL && pidf.len == 0) {
1035
+			LM_ERR("location object not found\n");
1036
+			goto err;
1037
+		}
1038
+		/* parse the pidf and get loc object */
1039
+		loc = lost_parse_pidf(pidf, urn);
1005 1040
 	}
1006
-	if(pidf.s == NULL && pidf.len == 0) {
1007
-		LM_ERR("location object not found\n");
1008
-		goto err;
1009
-	}
1010
-	/* parse the pidf and get loc object */
1011
-	loc = lost_parse_pidf(pidf, urn);
1041
+	/* pidf parsing failed ... return */
1012 1042
 	if(loc == NULL) {
1013 1043
 		LM_ERR("parsing pidf failed\n");
1014 1044
 		goto err;
... ...
@@ -1016,6 +1046,7 @@ int lost_function(struct sip_msg *_m, char *_con, char *_uri, char *_name,
1016 1046
 	/* assemble findService request */
1017 1047
 	req.s = lost_find_service_request(loc, &req.len);
1018 1048
 	lost_free_loc(&loc); /* clean up */
1049
+
1019 1050
 	if(req.s == NULL && req.len == 0) {
1020 1051
 		LM_ERR("lost request failed\n");
1021 1052
 		goto err;
... ...
@@ -1028,11 +1059,13 @@ int lost_function(struct sip_msg *_m, char *_con, char *_uri, char *_name,
1028 1059
 		/* copy url */
1029 1060
 		len = 0;
1030 1061
 		urlrep = lost_copy_string(url, &len);
1031
-		if(len > 0) {
1062
+		if(urlrep != NULL && len > 0) {
1032 1063
 			/* send request */
1033 1064
 			curl = httpapi.http_client_query(_m, urlrep, &ret, req.s, mtlost);
1034
-			pkg_free(urlrep); /*clean up */
1065
+			pkg_free(urlrep); /* clean up */
1066
+			urlrep = NULL;
1035 1067
 		} else {
1068
+			LM_ERR("could not copy POST url\n");
1036 1069
 			goto err;
1037 1070
 		}
1038 1071
 	} else {
... ...
@@ -1040,12 +1073,20 @@ int lost_function(struct sip_msg *_m, char *_con, char *_uri, char *_name,
1040 1073
 	}
1041 1074
 	/* only HTTP 2xx responses are accepted */
1042 1075
 	if(curl >= 300 || curl < 100) {
1043
-		LM_ERR("POST [%.*s] failed with error: %d\n", con.len, con.s, curl);
1076
+		if(naptr) {
1077
+			LM_ERR("POST [%.*s] failed with error: %d\n", url.len, url.s, curl);
1078
+		} else {
1079
+			LM_ERR("POST [%.*s] failed with error: %d\n", con.len, con.s, curl);
1080
+		}
1044 1081
 		lost_free_string(&ret);
1045 1082
 		goto err;
1046 1083
 	}
1047 1084
 
1048
-	LM_DBG("[%.*s] returned: %d\n", con.len, con.s, curl);
1085
+	if(naptr) {
1086
+		LM_DBG("[%.*s] returned: %d\n", url.len, url.s, curl);
1087
+	} else {
1088
+		LM_DBG("[%.*s] returned: %d\n", con.len, con.s, curl);
1089
+	}
1049 1090
 
1050 1091
 	if(ret.len == 0) {
1051 1092
 		LM_ERR("findService request failed\n");
... ...
@@ -1119,8 +1160,9 @@ int lost_function(struct sip_msg *_m, char *_con, char *_uri, char *_name,
1119 1160
 						tmp.len = strlen(fsrdata->redirect->target);
1120 1161
 						url.s = &(ustr[0]);
1121 1162
 						url.len = MAX_URI_SIZE;
1122
-						if((naptr = lost_naptr_lookup(tmp, &shttps, &url))
1123
-								== 0) {
1163
+						naptr = lost_naptr_lookup(tmp, &shttps, &url);
1164
+						if(naptr == 0) {
1165
+							/* fallback to http */
1124 1166
 							naptr = lost_naptr_lookup(tmp, &shttp, &url);
1125 1167
 						}
1126 1168
 						if(naptr == 0) {
... ...
@@ -1150,20 +1192,22 @@ int lost_function(struct sip_msg *_m, char *_con, char *_uri, char *_name,
1150 1192
 						/* copy url */
1151 1193
 						len = 0;
1152 1194
 						urlrep = lost_copy_string(url, &len);
1153
-						if(len > 0) {
1195
+						if(urlrep != NULL && len > 0) {
1154 1196
 							/* send request */
1155 1197
 							curl = httpapi.http_client_query(
1156 1198
 									_m, urlrep, &ret, req.s, mtlost);
1157 1199
 							url.s = NULL;
1158 1200
 							url.len = 0;
1159 1201
 							pkg_free(urlrep); /*clean up */
1202
+							urlrep = NULL;
1160 1203
 							/* only HTTP 2xx responses are accepted */
1161 1204
 							if(curl >= 300 || curl < 100) {
1162
-								LM_ERR("POST [%s] failed with error: %d\n",
1163
-										urlrep, curl);
1205
+								LM_ERR("POST [%.*s] failed with error: %d\n",
1206
+									url.len, url.s, curl);
1164 1207
 								goto err;
1165 1208
 							}
1166 1209
 						} else {
1210
+							LM_ERR("could not copy POST url\n");
1167 1211
 							goto err;
1168 1212
 						}
1169 1213
 						/* once more ... we got a redirect */
... ...
@@ -1197,6 +1241,7 @@ int lost_function(struct sip_msg *_m, char *_con, char *_uri, char *_name,
1197 1241
 	pvname.flags = PV_VAL_STR;
1198 1242
 	psname = (pv_spec_t *)_name;
1199 1243
 	psname->setf(_m, &psname->pvp, (int)EQ_T, &pvname);
1244
+	lost_free_string(&name); /* clean up */
1200 1245
 
1201 1246
 	pvuri.rs = uri;
1202 1247
 	pvuri.rs.s = uri.s;
... ...
@@ -1205,6 +1250,7 @@ int lost_function(struct sip_msg *_m, char *_con, char *_uri, char *_name,
1205 1250
 	pvuri.flags = PV_VAL_STR;
1206 1251
 	psuri = (pv_spec_t *)_uri;
1207 1252
 	psuri->setf(_m, &psuri->pvp, (int)EQ_T, &pvuri);
1253
+	lost_free_string(&uri); /* clean up */
1208 1254
 
1209 1255
 	pverr.rs = err;
1210 1256
 	pverr.rs.s = err.s;
... ...
@@ -1213,6 +1259,7 @@ int lost_function(struct sip_msg *_m, char *_con, char *_uri, char *_name,
1213 1259
 	pverr.flags = PV_VAL_STR;
1214 1260
 	pserr = (pv_spec_t *)_err;
1215 1261
 	pserr->setf(_m, &pserr->pvp, (int)EQ_T, &pverr);
1262
+	lost_free_string(&err); /* clean up */
1216 1263
 
1217 1264
 	return (err.len > 0) ? LOST_SERVER_ERROR : LOST_SUCCESS;
1218 1265
 
... ...
@@ -1230,6 +1277,15 @@ err:
1230 1277
 	if(req.s != NULL && req.len > 0) {
1231 1278
 		lost_free_string(&req);
1232 1279
 	}
1280
+	if(name.s != NULL && name.len > 0) {
1281
+		lost_free_string(&name);
1282
+	}
1283
+	if(uri.s != NULL && uri.len > 0) {
1284
+		lost_free_string(&uri);
1285
+	}
1286
+	if(err.s != NULL && err.len > 0) {
1287
+		lost_free_string(&err);
1288
+	}
1233 1289
 
1234 1290
 	return LOST_CLIENT_ERROR;
1235 1291
 }
... ...
@@ -176,6 +176,7 @@ static void destroy(void)
176 176
 {
177 177
 	if(held_loc_type.s != NULL && held_loc_type.len > 0) {
178 178
 		pkg_free(held_loc_type.s);
179
+		held_loc_type.s = NULL;
179 180
 		held_loc_type.len = 0;
180 181
 	}
181 182
 	/* do nothing */
... ...
@@ -211,8 +212,7 @@ static int fixup_lost_held_query(void **param, int param_no)
211 212
 static int fixup_free_lost_held_query(void **param, int param_no)
212 213
 {
213 214
 	if(param_no == 1) {
214
-		/* char strings don't need freeing */
215
-		return 0;
215
+		return fixup_spve_null(param, 1);	
216 216
 	}
217 217
 	if((param_no == 2) || (param_no == 3) || (param_no == 4)) {
218 218
 		return fixup_free_pvar_null(param, 1);
... ...
@@ -232,6 +232,10 @@ void lost_free_loc(p_lost_loc_t *loc)
232 232
 
233 233
 	pkg_free(ptr);
234 234
 	*loc = NULL;
235
+
236
+	LM_DBG("### location object removed\n");
237
+
238
+	return;
235 239
 }
236 240
 
237 241
 /*
... ...
@@ -242,7 +246,7 @@ void lost_free_held(p_lost_held_t *held)
242 246
 {
243 247
 	p_lost_held_t ptr;
244 248
 
245
-	if(held == NULL)
249
+	if(*held == NULL)
246 250
 		return;
247 251
 
248 252
 	ptr = *held;
... ...
@@ -254,6 +258,10 @@ void lost_free_held(p_lost_held_t *held)
254 258
 
255 259
 	pkg_free(ptr);
256 260
 	*held = NULL;
261
+
262
+	LM_DBG("### location-request object removed\n");
263
+
264
+	return;
257 265
 }
258 266
 
259 267
 /*
... ...
@@ -275,7 +283,7 @@ char *lost_copy_string(str src, int *lgth)
275 283
 		*lgth = 0;
276 284
 	} else {
277 285
 		memset(res, 0, src.len + 1);
278
-		memcpy(res, src.s, src.len + 1);
286
+		memcpy(res, src.s, src.len);
279 287
 		res[src.len] = '\0';
280 288
 		*lgth = (int)strlen(res);
281 289
 	}
... ...
@@ -298,6 +306,9 @@ void lost_free_string(str *string)
298 306
 
299 307
 	if(ptr.s != NULL && ptr.len > 0) {
300 308
 		pkg_free(ptr.s);
309
+
310
+		LM_DBG("### string object removed\n");
311
+	
301 312
 	}
302 313
 
303 314
 	string->s = NULL;
... ...
@@ -684,6 +695,8 @@ void lost_free_geoheader_list(p_lost_geolist_t *list)
684 695
 
685 696
 	*list = NULL;
686 697
 
698
+	LM_DBG("### geoheader list removed\n");
699
+
687 700
 	return;
688 701
 }
689 702