From 0346fda0b3ddc484c3a28d88a5c42b890feb3079 Mon Sep 17 00:00:00 2001 From: Dmitry Podgorny Date: Sat, 3 Aug 2013 14:27:07 +0300 Subject: most FREE_SET_NULL replaced with free FREE_SET_NULL makes extra assignment of NULL for pointers in stack or dynamic memory that is going to be freed. FREE_SET_NULL is useful for pointers that can be used in future. --- src/xmpp/capabilities.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'src/xmpp/capabilities.c') diff --git a/src/xmpp/capabilities.c b/src/xmpp/capabilities.c index 10aa8a38..f9624f90 100644 --- a/src/xmpp/capabilities.c +++ b/src/xmpp/capabilities.c @@ -301,17 +301,16 @@ static void _caps_destroy(Capabilities *caps) { if (caps != NULL) { - FREE_SET_NULL(caps->category); - FREE_SET_NULL(caps->type); - FREE_SET_NULL(caps->name); - FREE_SET_NULL(caps->software); - FREE_SET_NULL(caps->software_version); - FREE_SET_NULL(caps->os); - FREE_SET_NULL(caps->os_version); + free(caps->category); + free(caps->type); + free(caps->name); + free(caps->software); + free(caps->software_version); + free(caps->os); + free(caps->os_version); if (caps->features != NULL) { g_slist_free_full(caps->features, free); - caps->features = NULL; } - FREE_SET_NULL(caps); + free(caps); } } -- cgit 1.4.1-2-gfad0 From a6e66cc57106402944445fc709625dcf88785a5b Mon Sep 17 00:00:00 2001 From: Dmitry Podgorny Date: Sat, 3 Aug 2013 14:38:38 +0300 Subject: fixed memory leaks Also avoided several NULL pointer dereferences. --- src/ui/notifier.c | 6 ++--- src/xmpp/capabilities.c | 32 +++++++++++++------------- src/xmpp/iq.c | 20 +++++++++-------- src/xmpp/message.c | 6 +++++ src/xmpp/presence.c | 60 ++++++++++++++++++++++++++++++++++++++++--------- 5 files changed, 85 insertions(+), 39 deletions(-) (limited to 'src/xmpp/capabilities.c') diff --git a/src/ui/notifier.c b/src/ui/notifier.c index f43c8c25..403c215e 100644 --- a/src/ui/notifier.c +++ b/src/ui/notifier.c @@ -80,7 +80,7 @@ notify_invite(const char * const from, const char * const room, _notify(message->str, 10000, "Incoming message"); - g_string_free(message, FALSE); + g_string_free(message, TRUE); } void @@ -102,7 +102,7 @@ notify_room_message(const char * const handle, const char * const room, int win) _notify(text->str, 10000, "incoming message"); - g_string_free(text, FALSE); + g_string_free(text, TRUE); } void @@ -111,7 +111,7 @@ notify_subscription(const char * const from) GString *message = g_string_new("Subscription request: \n"); g_string_append(message, from); _notify(message->str, 10000, "Incomming message"); - g_string_free(message, FALSE); + g_string_free(message, TRUE); } void diff --git a/src/xmpp/capabilities.c b/src/xmpp/capabilities.c index f9624f90..ed3cf169 100644 --- a/src/xmpp/capabilities.c +++ b/src/xmpp/capabilities.c @@ -122,7 +122,7 @@ caps_create_sha1_str(xmpp_stanza_t * const query) GSList *form_names = NULL; DataForm *form = NULL; FormField *field = NULL; - GHashTable *forms = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)stanza_destroy_form); + GHashTable *forms = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, (GDestroyNotify)stanza_destroy_form); GString *s = g_string_new(""); @@ -134,18 +134,18 @@ caps_create_sha1_str(xmpp_stanza_t * const query) lang = xmpp_stanza_get_attribute(child, "xml:lang"); name = xmpp_stanza_get_attribute(child, "name"); - GString *identity_str = g_string_new(g_strdup(category)); + GString *identity_str = g_string_new(category); g_string_append(identity_str, "/"); if (type != NULL) { - g_string_append(identity_str, g_strdup(type)); + g_string_append(identity_str, type); } g_string_append(identity_str, "/"); if (lang != NULL) { - g_string_append(identity_str, g_strdup(lang)); + g_string_append(identity_str, lang); } g_string_append(identity_str, "/"); if (name != NULL) { - g_string_append(identity_str, g_strdup(name)); + g_string_append(identity_str, name); } g_string_append(identity_str, "<"); identities = g_slist_insert_sorted(identities, g_strdup(identity_str->str), (GCompareFunc)octet_compare); @@ -156,8 +156,8 @@ caps_create_sha1_str(xmpp_stanza_t * const query) } else if (g_strcmp0(xmpp_stanza_get_name(child), STANZA_NAME_X) == 0) { if (strcmp(xmpp_stanza_get_ns(child), STANZA_NS_DATA) == 0) { form = stanza_create_form(child); - form_names = g_slist_insert_sorted(form_names, strdup(form->form_type), (GCompareFunc)octet_compare); - g_hash_table_insert(forms, strdup(form->form_type), form); + form_names = g_slist_insert_sorted(form_names, g_strdup(form->form_type), (GCompareFunc)octet_compare); + g_hash_table_insert(forms, g_strdup(form->form_type), form); } } child = xmpp_stanza_get_next(child); @@ -165,13 +165,13 @@ caps_create_sha1_str(xmpp_stanza_t * const query) GSList *curr = identities; while (curr != NULL) { - g_string_append(s, strdup(curr->data)); + g_string_append(s, curr->data); curr = g_slist_next(curr); } curr = features; while (curr != NULL) { - g_string_append(s, strdup(curr->data)); + g_string_append(s, curr->data); g_string_append(s, "<"); curr = g_slist_next(curr); } @@ -179,17 +179,17 @@ caps_create_sha1_str(xmpp_stanza_t * const query) curr = form_names; while (curr != NULL) { form = g_hash_table_lookup(forms, curr->data); - g_string_append(s, strdup(form->form_type)); + g_string_append(s, form->form_type); g_string_append(s, "<"); GSList *curr_field = form->fields; while (curr_field != NULL) { field = curr_field->data; - g_string_append(s, strdup(field->var)); + g_string_append(s, field->var); g_string_append(s, "<"); GSList *curr_value = field->values; while (curr_value != NULL) { - g_string_append(s, strdup(curr_value->data)); + g_string_append(s, curr_value->data); g_string_append(s, "<"); curr_value = g_slist_next(curr_value); } @@ -215,10 +215,10 @@ caps_create_sha1_str(xmpp_stanza_t * const query) char *result = g_base64_encode(md_value, md_len); g_string_free(s, TRUE); - g_slist_free_full(identities, free); - g_slist_free_full(features, free); - g_slist_free_full(form_names, free); - //g_hash_table_destroy(forms); + g_slist_free_full(identities, g_free); + g_slist_free_full(features, g_free); + g_slist_free_full(form_names, g_free); + g_hash_table_destroy(forms); return result; } diff --git a/src/xmpp/iq.c b/src/xmpp/iq.c index 48b561f0..742a2258 100644 --- a/src/xmpp/iq.c +++ b/src/xmpp/iq.c @@ -312,10 +312,10 @@ static void _identity_destroy(DiscoIdentity *identity) { if (identity != NULL) { - FREE_SET_NULL(identity->name); - FREE_SET_NULL(identity->type); - FREE_SET_NULL(identity->category); - FREE_SET_NULL(identity); + free(identity->name); + free(identity->type); + free(identity->category); + free(identity); } } @@ -323,9 +323,9 @@ static void _item_destroy(DiscoItem *item) { if (item != NULL) { - FREE_SET_NULL(item->jid); - FREE_SET_NULL(item->name); - FREE_SET_NULL(item); + free(item->jid); + free(item->name); + free(item); } } @@ -411,12 +411,13 @@ _iq_handle_discoinfo_result(xmpp_conn_t * const conn, xmpp_stanza_t * const stan log_info("Generated sha-1 does not match given:"); log_info("Generated : %s", generated_sha1); log_info("Given : %s", given_sha1); - FREE_SET_NULL(generated_sha1); + g_free(generated_sha1); g_strfreev(split); + free(caps_key); return 1; } - FREE_SET_NULL(generated_sha1); + g_free(generated_sha1); g_strfreev(split); // non supported hash, or legacy caps @@ -429,6 +430,7 @@ _iq_handle_discoinfo_result(xmpp_conn_t * const conn, xmpp_stanza_t * const stan // already cached if (caps_contains(caps_key)) { log_info("Client info already cached."); + free(caps_key); return 1; } diff --git a/src/xmpp/message.c b/src/xmpp/message.c index 29d11958..95b3152a 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -212,6 +212,9 @@ _conference_message_handler(xmpp_conn_t * const conn, } Jid *jidp = jid_create(invitor_jid); + if (jidp == NULL) { + return 1; + } invitor = jidp->barejid; xmpp_stanza_t *reason_st = xmpp_stanza_get_child_by_name(invite, STANZA_NAME_REASON); @@ -233,6 +236,9 @@ _conference_message_handler(xmpp_conn_t * const conn, } Jid *jidp = jid_create(from); + if (jidp == NULL) { + return 1; + } invitor = jidp->barejid; reason = xmpp_stanza_get_attribute(x_groupchat, STANZA_ATTR_REASON); diff --git a/src/xmpp/presence.c b/src/xmpp/presence.c index 6c87a8ac..93870088 100644 --- a/src/xmpp/presence.c +++ b/src/xmpp/presence.c @@ -156,14 +156,23 @@ presence_sub_request_find(char * search_str) gboolean presence_sub_request_exists(const char * const bare_jid) { - GSList *requests = autocomplete_get_list(sub_requests_ac); + gboolean result = FALSE; + GSList *requests_p = autocomplete_get_list(sub_requests_ac); + GSList *requests = requests_p; + while (requests != NULL) { if (strcmp(requests->data, bare_jid) == 0) { - return TRUE; + result = TRUE; + break; } requests = g_slist_next(requests); } - return FALSE; + + if (requests_p != NULL) { + g_slist_free_full(requests_p, free); + } + + return result; } void @@ -220,20 +229,28 @@ presence_update(const resource_presence_t presence_type, const char * const msg, static void _send_room_presence(xmpp_conn_t *conn, xmpp_stanza_t *presence) { - GList *rooms = muc_get_active_room_list(); + GList *rooms_p = muc_get_active_room_list(); + GList *rooms = rooms_p; + while (rooms != NULL) { const char *room = rooms->data; const char *nick = muc_get_room_nick(room); - char *full_room_jid = create_fulljid(room, nick); - xmpp_stanza_set_attribute(presence, STANZA_ATTR_TO, full_room_jid); - log_debug("Sending presence to room: %s", full_room_jid); - xmpp_send(conn, presence); - free(full_room_jid); + if (nick != NULL) { + char *full_room_jid = create_fulljid(room, nick); + + xmpp_stanza_set_attribute(presence, STANZA_ATTR_TO, full_room_jid); + log_debug("Sending presence to room: %s", full_room_jid); + xmpp_send(conn, presence); + free(full_room_jid); + } rooms = g_list_next(rooms); } - g_list_free(rooms); + + if (rooms_p != NULL) { + g_list_free(rooms_p); + } } void @@ -347,9 +364,13 @@ _subscribe_handler(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza, void * const userdata) { char *from = xmpp_stanza_get_attribute(stanza, STANZA_ATTR_FROM); - Jid *from_jid = jid_create(from); log_debug("Subscribe presence handler fired for %s", from); + Jid *from_jid = jid_create(from); + if (from_jid == NULL) { + return 1; + } + prof_handle_subscription(from_jid->barejid, PRESENCE_SUBSCRIBE); autocomplete_add(sub_requests_ac, strdup(from_jid->barejid)); @@ -368,6 +389,11 @@ _unavailable_handler(xmpp_conn_t * const conn, Jid *my_jid = jid_create(jid); Jid *from_jid = jid_create(from); + if (my_jid == NULL || from_jid == NULL) { + jid_destroy(my_jid); + jid_destroy(from_jid); + return 1; + } char *status_str = stanza_get_status(stanza, NULL); @@ -420,6 +446,11 @@ _available_handler(xmpp_conn_t * const conn, Jid *my_jid = jid_create(jid); Jid *from_jid = jid_create(from); + if (my_jid == NULL || from_jid == NULL) { + jid_destroy(my_jid); + jid_destroy(from_jid); + return 1; + } char *show_str = stanza_get_show(stanza, "online"); char *status_str = stanza_get_status(stanza, NULL); @@ -515,6 +546,10 @@ _get_caps_key(xmpp_stanza_t * const stanza) log_debug("Presence contains capabilities."); + if (node == NULL) { + return NULL; + } + // xep-0115 if ((hash_type != NULL) && (strcmp(hash_type, "sha-1") == 0)) { log_debug("Hash type %s supported.", hash_type); @@ -544,6 +579,8 @@ _get_caps_key(xmpp_stanza_t * const stanza) g_string_free(id_str, TRUE); } + g_free(node); + return caps_key; } @@ -626,6 +663,7 @@ _room_presence_handler(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza, if (old_nick != NULL) { muc_add_to_roster(room, nick, show_str, status_str, caps_key); prof_handle_room_member_nick_change(room, old_nick, nick); + free(old_nick); } else { if (!muc_nick_in_roster(room, nick)) { prof_handle_room_member_online(room, nick, show_str, status_str, caps_key); -- cgit 1.4.1-2-gfad0 From b1bfdf650a93c84b92fef02fdd8441c1b2002e66 Mon Sep 17 00:00:00 2001 From: Dmitry Podgorny Date: Sun, 25 Aug 2013 14:54:34 +0300 Subject: fixed memory leaks in various files --- src/config/accounts.c | 61 ++++++++++++++++++++++++++++++++++--------------- src/xmpp/capabilities.c | 9 ++++---- src/xmpp/iq.c | 15 ++++++++---- src/xmpp/message.c | 1 + src/xmpp/presence.c | 1 + 5 files changed, 59 insertions(+), 28 deletions(-) (limited to 'src/xmpp/capabilities.c') diff --git a/src/config/accounts.c b/src/config/accounts.c index c82f6b2d..3d198874 100644 --- a/src/config/accounts.c +++ b/src/config/accounts.c @@ -72,10 +72,7 @@ accounts_load(void) _fix_legacy_accounts(account_names[i]); } - for (i = 0; i < naccounts; i++) { - free(account_names[i]); - } - free(account_names); + g_strfreev(account_names); } void @@ -166,6 +163,7 @@ accounts_get_account(const char * const name) gchar *jid = g_key_file_get_string(accounts, name, "jid", NULL); if (jid != NULL) { account->jid = strdup(jid); + g_free(jid); } else { account->jid = strdup(name); g_key_file_set_string(accounts, name, "jid", name); @@ -177,6 +175,7 @@ accounts_get_account(const char * const name) gchar *server = g_key_file_get_string(accounts, name, "server", NULL); if (server != NULL) { account->server = strdup(server); + g_free(server); } else { account->server = NULL; } @@ -184,6 +183,7 @@ accounts_get_account(const char * const name) gchar *resource = g_key_file_get_string(accounts, name, "resource", NULL); if (resource != NULL) { account->resource = strdup(resource); + g_free(resource); } else { account->resource = NULL; } @@ -195,6 +195,10 @@ accounts_get_account(const char * const name) account->last_presence = strdup(presence); } + if (presence != NULL) { + g_free(presence); + } + presence = g_key_file_get_string(accounts, name, "presence.login", NULL); if (presence == NULL) { account->login_presence = strdup("online"); @@ -206,6 +210,10 @@ accounts_get_account(const char * const name) account->login_presence = strdup(presence); } + if (presence != NULL) { + g_free(presence); + } + account->priority_online = g_key_file_get_integer(accounts, name, "priority.online", NULL); account->priority_chat = g_key_file_get_integer(accounts, name, "priority.chat", NULL); account->priority_away = g_key_file_get_integer(accounts, name, "priority.away", NULL); @@ -302,7 +310,7 @@ accounts_rename(const char * const account_name, const char * const new_name) char *value = g_key_file_get_string(accounts, account_name, string_keys[i], NULL); if (value != NULL) { g_key_file_set_string(accounts, new_name, string_keys[i], value); - free(value); + g_free(value); } } @@ -469,45 +477,59 @@ accounts_set_login_presence(const char * const account_name, const char * const resource_presence_t accounts_get_last_presence(const char * const account_name) { + resource_presence_t result; gchar *setting = g_key_file_get_string(accounts, account_name, "presence.last", NULL); + if (setting == NULL || (strcmp(setting, "online") == 0)) { - return RESOURCE_ONLINE; + result = RESOURCE_ONLINE; } else if (strcmp(setting, "chat") == 0) { - return RESOURCE_CHAT; + result = RESOURCE_CHAT; } else if (strcmp(setting, "away") == 0) { - return RESOURCE_AWAY; + result = RESOURCE_AWAY; } else if (strcmp(setting, "xa") == 0) { - return RESOURCE_XA; + result = RESOURCE_XA; } else if (strcmp(setting, "dnd") == 0) { - return RESOURCE_DND; + result = RESOURCE_DND; } else { log_warning("Error reading presence.last for account: '%s', value: '%s', defaulting to 'online'", account_name, setting); - return RESOURCE_ONLINE; + result = RESOURCE_ONLINE; } + + if (setting != NULL) { + g_free(setting); + } + return result; } resource_presence_t accounts_get_login_presence(const char * const account_name) { + resource_presence_t result; gchar *setting = g_key_file_get_string(accounts, account_name, "presence.login", NULL); + if (setting == NULL || (strcmp(setting, "online") == 0)) { - return RESOURCE_ONLINE; + result = RESOURCE_ONLINE; } else if (strcmp(setting, "chat") == 0) { - return RESOURCE_CHAT; + result = RESOURCE_CHAT; } else if (strcmp(setting, "away") == 0) { - return RESOURCE_AWAY; + result = RESOURCE_AWAY; } else if (strcmp(setting, "xa") == 0) { - return RESOURCE_XA; + result = RESOURCE_XA; } else if (strcmp(setting, "dnd") == 0) { - return RESOURCE_DND; + result = RESOURCE_DND; } else if (strcmp(setting, "last") == 0) { - return accounts_get_last_presence(account_name); + result = accounts_get_last_presence(account_name); } else { log_warning("Error reading presence.login for account: '%s', value: '%s', defaulting to 'online'", account_name, setting); - return RESOURCE_ONLINE; + result = RESOURCE_ONLINE; } + + if (setting != NULL) { + g_free(setting); + } + return result; } static void @@ -543,8 +565,9 @@ static void _save_accounts(void) { gsize g_data_size; - char *g_accounts_data = g_key_file_to_data(accounts, &g_data_size, NULL); + gchar *g_accounts_data = g_key_file_to_data(accounts, &g_data_size, NULL); g_file_set_contents(accounts_loc, g_accounts_data, g_data_size, NULL); + g_free(g_accounts_data); } static gchar * diff --git a/src/xmpp/capabilities.c b/src/xmpp/capabilities.c index ed3cf169..2b0a12e5 100644 --- a/src/xmpp/capabilities.c +++ b/src/xmpp/capabilities.c @@ -280,13 +280,14 @@ caps_create_query_response_stanza(xmpp_ctx_t * const ctx) xmpp_stanza_add_child(query, feature_version); xmpp_stanza_add_child(query, feature_ping); - xmpp_stanza_release(identity); + xmpp_stanza_release(feature_ping); + xmpp_stanza_release(feature_version); xmpp_stanza_release(feature_muc); - xmpp_stanza_release(feature_discoinfo); xmpp_stanza_release(feature_discoitems); - xmpp_stanza_release(feature_caps); - xmpp_stanza_release(feature_version); + xmpp_stanza_release(feature_discoinfo); xmpp_stanza_release(feature_chatstates); + xmpp_stanza_release(feature_caps); + xmpp_stanza_release(identity); return query; } diff --git a/src/xmpp/iq.c b/src/xmpp/iq.c index 3430b4d1..5de84056 100644 --- a/src/xmpp/iq.c +++ b/src/xmpp/iq.c @@ -247,6 +247,12 @@ _iq_handle_version_get(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza, xmpp_send(conn, response); + g_free(version_str); + xmpp_stanza_release(name_txt); + xmpp_stanza_release(version_txt); + xmpp_stanza_release(name); + xmpp_stanza_release(version); + xmpp_stanza_release(query); xmpp_stanza_release(response); } @@ -437,9 +443,6 @@ _iq_handle_discoinfo_result(xmpp_conn_t * const conn, xmpp_stanza_t * const stan log_debug("Client info not cached"); - DataForm *form = NULL; - FormField *formField = NULL; - const char *category = NULL; const char *type = NULL; const char *name = NULL; @@ -458,7 +461,8 @@ _iq_handle_discoinfo_result(xmpp_conn_t * const conn, xmpp_stanza_t * const stan xmpp_stanza_t *softwareinfo = xmpp_stanza_get_child_by_ns(query, STANZA_NS_DATA); if (softwareinfo != NULL) { - form = stanza_create_form(softwareinfo); + DataForm *form = stanza_create_form(softwareinfo); + FormField *formField = NULL; if (g_strcmp0(form->form_type, STANZA_DATAFORM_SOFTWARE) == 0) { GSList *field = form->fields; @@ -478,6 +482,8 @@ _iq_handle_discoinfo_result(xmpp_conn_t * const conn, xmpp_stanza_t * const stan field = g_slist_next(field); } } + + stanza_destroy_form(form); } xmpp_stanza_t *child = xmpp_stanza_get_children(query); @@ -492,7 +498,6 @@ _iq_handle_discoinfo_result(xmpp_conn_t * const conn, xmpp_stanza_t * const stan caps_add(caps_key, category, type, name, software, software_version, os, os_version, features); - //stanza_destroy_form(form); free(caps_key); } diff --git a/src/xmpp/message.c b/src/xmpp/message.c index 95b3152a..19b4df49 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -280,6 +280,7 @@ _groupchat_message_handler(xmpp_conn_t * const conn, message = xmpp_stanza_get_text(subject); if (message != NULL) { prof_handle_room_subject(jid->barejid, message); + xmpp_free(ctx, message); } jid_destroy(jid); diff --git a/src/xmpp/presence.c b/src/xmpp/presence.c index a1d851e4..ec439871 100644 --- a/src/xmpp/presence.c +++ b/src/xmpp/presence.c @@ -502,6 +502,7 @@ _available_handler(xmpp_conn_t * const conn, last_activity); } + free(caps_key); free(status_str); free(show_str); jid_destroy(my_jid); -- cgit 1.4.1-2-gfad0