Hi,
In relation to the thread "Memory leak in smsbox", here's a patch
which corrects some of the errors.
The patch reorganizes the error goto-fields so they are as followed:
fielderror1:
At this point, only receiver have been allocated
fielderror2:
receiver, allowed, and denied have been allocated (allowed and denied
may contain elements from receiver)
fielderror3:
receiver, allowed, denied, newfrom, and msg have been allocated
(allowed and denied contains elements from receiver)
error:
receiver, allowed, denied, newfrom, and failed_id have been allocated
(allowed is empty, and denied and failed_id may contain elements from
receiver)
Besides, the "Number(s) has/have been denied by white- and/or
black-lists." error have been moved to before SMS transmission is
attempted, and the logic is now gwlist_len(allowed) == 0 (No recipient
have been allowed)
This patch overrules my original patch for the memory leak.
For the record; While monitoring its memory consumption, I've flooded
smsbox with messages for each possible error condition (except
conditions where recipients are added to the failed_id list. Don't
know how to provoke that condition), and there seems to be no memory
leaks, double frees, or segmentation faults.
------------------------------------------------------------------------
diff -Nru gateway/gw/smsbox.c gateway.smsbox/gw/smsbox.c
--- gateway/gw/smsbox.c 2005-12-09 03:14:31.000000000 +0100
+++ gateway.smsbox/gw/smsbox.c 2006-01-23 17:51:55.000000000 +0100
@@ -1990,11 +1990,11 @@
*/
if (udh != NULL && (octstr_len(udh) != octstr_get_char(udh, 0) +
1)) {
returnerror = octstr_create("UDH field misformed, rejected");
- goto fielderror2;
+ goto fielderror1;
}
if (udh != NULL && octstr_len(udh) > MAX_SMS_OCTETS) {
returnerror = octstr_create("UDH field is too long, rejected");
- goto fielderror2;
+ goto fielderror1;
}
/*
@@ -2108,6 +2108,12 @@
del = gwlist_delete_matching(allowed, receiv,
octstr_item_match);
}
+ /* have all receivers been denied by list rules?! */
+ if (gwlist_len(allowed) == 0) {
+ returnerror = octstr_create("Number(s) has/have been denied
by white- and/or black-lists.");
+ goto fielderror2;
+ }
+
if (urltrans_faked_sender(t) != NULL) {
/* discard previous from */
newfrom = octstr_duplicate(urltrans_faked_sender(t));
@@ -2144,7 +2150,7 @@
msg->sms.account = account ? octstr_duplicate(account) : NULL;
} else {
returnerror = octstr_create("Account field misformed,
rejected");
- goto fielderror;
+ goto fielderror3;
}
}
msg->sms.msgdata = text ? octstr_duplicate(text) :
octstr_create("");
@@ -2156,7 +2162,7 @@
if(octstr_len(dlr_url)) {
if(octstr_len(dlr_url) < 8) { /* http(s):// */
returnerror = octstr_create("DLR-URL field misformed,
rejected");
- goto fielderror;
+ goto fielderror3;
} else {
Octstr *tmp;
tmp = octstr_copy(dlr_url, 0, 7);
@@ -2168,7 +2174,7 @@
if(octstr_case_compare(tmp, octstr_imm("https://";)) != 0) {
returnerror = octstr_create("DLR-URL field misformed,
rejected");
O_DESTROY(tmp);
- goto fielderror;
+ goto fielderror3;
}
#ifdef HAVE_LIBSSL
msg->sms.dlr_url = octstr_duplicate(dlr_url);
@@ -2187,49 +2193,49 @@
if ( dlr_mask < -1 || dlr_mask > 31 ) { /* 00011111 */
returnerror = octstr_create("DLR-Mask field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.dlr_mask = dlr_mask;
if ( mclass < -1 || mclass > 3 ) {
returnerror = octstr_create("MClass field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.mclass = mclass;
if ( pid < -1 || pid > 255 ) {
returnerror = octstr_create("PID field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.pid = pid;
if ( rpi < -1 || rpi > 2) {
returnerror = octstr_create("RPI field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.rpi = rpi;
if ( alt_dcs < -1 || alt_dcs > 1 ) {
returnerror = octstr_create("Alt-DCS field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.alt_dcs = alt_dcs;
if ( mwi < -1 || mwi > 7 ) {
returnerror = octstr_create("MWI field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.mwi = mwi;
if ( coding < -1 || coding > 2 ) {
returnerror = octstr_create("Coding field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.coding = coding;
if ( compress < -1 || compress > 1 ) {
returnerror = octstr_create("Compress field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.compress = compress;
@@ -2244,19 +2250,19 @@
if ( validity < -1 ) {
returnerror = octstr_create("Validity field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.validity = validity;
if ( deferred < -1 ) {
returnerror = octstr_create("Deferred field misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.deferred = deferred;
if (priority != SMS_PARAM_UNDEFINED && (priority < 0 ||
priority > 3)) {
returnerror = octstr_create("Priority field misformed,
rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.priority = priority;
@@ -2279,7 +2285,7 @@
if (charset_processing(charset, msg->sms.msgdata,
msg->sms.coding) == -1) {
returnerror = octstr_create("Charset or body misformed, rejected");
- goto fielderror;
+ goto fielderror3;
}
msg->sms.receiver = NULL;
@@ -2322,19 +2328,12 @@
msg_destroy(msg);
- gwlist_destroy(receiver, octstr_destroy_item);
- gwlist_destroy(allowed, octstr_destroy_item);
-
- /* have all receivers been denied by list rules?! */
- if (no_recv == gwlist_len(denied)) {
- returnerror = octstr_create("Number(s) has/have been denied
by white- and/or black-lists.");
- goto fielderror2;
- }
if (gwlist_len(failed_id) > 0)
goto error;
- gwlist_destroy(failed_id, octstr_destroy_item);
+ gwlist_destroy(failed_id, NULL);
+ gwlist_destroy(allowed, NULL);
octstr_destroy(newfrom);
*status = HTTP_ACCEPTED;
returnerror = octstr_create("Sent.");
@@ -2348,8 +2347,9 @@
while ((receiv = gwlist_extract_first(denied)) != NULL) {
octstr_format_append(returnerror, " %s",
octstr_get_cstr(receiv));
}
- } - gwlist_destroy(denied,
octstr_destroy_item); + }
+ gwlist_destroy(denied, NULL);
+ gwlist_destroy(receiver, octstr_destroy_item);
/*
* Append number of splits to returned body. @@ -2361,11 +2361,17 @@
return returnerror;
-fielderror:
- octstr_destroy(newfrom);
+fielderror3:
msg_destroy(msg);
-
+ octstr_destroy(newfrom);
+ fielderror2:
+ gwlist_destroy(allowed, NULL);
+ gwlist_destroy(denied, NULL);
+
+fielderror1:
+ gwlist_destroy(receiver, octstr_destroy_item);
+ alog("send-SMS request failed - %s",
octstr_get_cstr(returnerror));
@@ -2374,7 +2380,6 @@
error:
error(0, "sendsms_request: failed");
- octstr_destroy(from);
*status = HTTP_INTERNAL_SERVER_ERROR;
returnerror = octstr_create("Sending failed.");
@@ -2389,9 +2394,11 @@
}
}
- octstr_destroy(receiv); - gwlist_destroy(failed_id,
octstr_destroy_item);
- gwlist_destroy(denied, octstr_destroy_item);
+ gwlist_destroy(failed_id, NULL);
+ gwlist_destroy(allowed, NULL);
+ gwlist_destroy(denied, NULL);
+ gwlist_destroy(receiver, octstr_destroy_item);
+ octstr_destroy(newfrom);
return returnerror;
}
