Re: PATCH: Memory leak in Clickatell + Brunet HTTP SMSC
Stipe Tolj <[email protected]>
| Newsgroups | gmane.comp.mobile.kannel.devel |
|---|---|
| Organization | tolj.org system architecture |
| Message-ID | <[email protected]> |
Donald Jackson schrieb:
> Hi everyone,
>
> Attached is a diff for memory leak found in smsc_http.c for Clickatell
> and Brunet specific implementations. The message ID Octstr's never get
> destroyed when the dictionary gets destroyed.
-1 on this patch. Reason: we do a double free() on the Octstr pointers and cause
a segfault.
The issue is:
- octstr_split_words() malloc()s the new items in the list.
- then we inject to Dict the pointer refrence, that's the "original", not a copy.
- then we gwlist_destroy() and hence all items of the list are freed, also
the one we put into the dict.
- later on we dict_destroy() _WITH_ the octstr_destroy() callback and hence
we'll try to double free. Bang.
Attached is a revised patch, that takes care we duplicate the items that we put
into the Dict hash, so we have a clean destruction later, as the
gwlist_destroy() took care of the original ones.
Please review and vote.
Stipe
-------------------------------------------------------------------
Kölner Landstrasse 419
40589 Düsseldorf, NRW, Germany
tolj.org system architecture Kannel Software Foundation (KSF)
http://www.tolj.org/ http://www.kannel.org/
mailto:st_{at}_tolj.org mailto:stolj_{at}_kannel.org
-------------------------------------------------------------------
smsc_http.diff
(text/plain, 2.2 KB)
### Eclipse Workspace Patch 1.0
#P gateway-cvs-head
Index: gw/smsc/smsc_http.c
===================================================================
RCS file: /home/cvs/gateway/gw/smsc/smsc_http.c,v
retrieving revision 1.56
diff -u -r1.56 smsc_http.c
--- gw/smsc/smsc_http.c 14 Jul 2008 14:50:12 -0000 1.56
+++ gw/smsc/smsc_http.c 14 Jul 2008 15:09:12 -0000
@@ -726,16 +726,16 @@
words = octstr_split_words(body);
if ((len = gwlist_len(words)) > 1) {
- word = gwlist_extract_first(words);
- if (octstr_compare(word, octstr_imm("ID:")) == 0) {
- value = gwlist_extract_first(words);
- param = dict_create(4, NULL);
- dict_put(param, octstr_imm("ID"), value);
- } else if (octstr_compare(word, octstr_imm("ERR:")) == 0) {
- value = gwlist_extract_first(words);
- param = dict_create(4, NULL);
- dict_put(param, octstr_imm("ERR"), value);
- }
+ word = gwlist_extract_first(words);
+ if (octstr_compare(word, octstr_imm("ID:")) == 0) {
+ value = gwlist_extract_first(words);
+ param = dict_create(4, (void(*)(void *)) octstr_destroy);
+ dict_put(param, octstr_imm("ID"), octstr_duplicate(value));
+ } else if (octstr_compare(word, octstr_imm("ERR:")) == 0) {
+ value = gwlist_extract_first(words);
+ param = dict_create(4, (void(*)(void *)) octstr_destroy);
+ dict_put(param, octstr_imm("ERR"), octstr_duplicate(value));
+ }
octstr_destroy(word);
}
gwlist_destroy(words, (void(*)(void *)) octstr_destroy);
@@ -1017,13 +1017,13 @@
words = octstr_split_words(body);
if ((len = gwlist_len(words)) > 0) {
- param = dict_create(4, NULL);
+ param = dict_create(4, (void(*)(void *)) octstr_destroy);
while ((word = gwlist_extract_first(words)) != NULL) {
List *l = octstr_split(word, octstr_imm("="));
Octstr *key = gwlist_extract_first(l);
Octstr *value = gwlist_extract_first(l);
if (octstr_len(key))
- dict_put(param, key, value);
+ dict_put(param, key, octstr_duplicate(value));
octstr_destroy(key);
octstr_destroy(word);
gwlist_destroy(l, (void(*)(void *)) octstr_destroy);