From 5505387c37f594db0368317a2349c429c8ddedfe Mon Sep 17 00:00:00 2001 From: Dmitry Podgorny Date: Wed, 31 Jul 2013 01:51:07 +0300 Subject: [PATCH 1/3] fixed several memory leaks in command.c and jid.c --- src/command/command.c | 59 ++++++++++++++++++++++++++++++------------- src/jid.c | 44 ++++++++++++++++---------------- 2 files changed, 63 insertions(+), 40 deletions(-) diff --git a/src/command/command.c b/src/command/command.c index 2e31605d..d4851116 100644 --- a/src/command/command.c +++ b/src/command/command.c @@ -1121,7 +1121,6 @@ cmd_execute_default(const char * const inp) ui_current_print_line("You are not currently connected."); } else { message_send_groupchat(inp, recipient); - free(recipient); } break; @@ -1139,7 +1138,6 @@ cmd_execute_default(const char * const inp) } ui_outgoing_msg("me", recipient, inp); - free(recipient); } break; @@ -1149,7 +1147,6 @@ cmd_execute_default(const char * const inp) } else { message_send(inp, recipient); ui_outgoing_msg("me", recipient, inp); - free(recipient); } break; @@ -1163,7 +1160,6 @@ cmd_execute_default(const char * const inp) } else { message_send_duck(inp); ui_duck(inp); - free(recipient); } break; @@ -1171,6 +1167,8 @@ cmd_execute_default(const char * const inp) break; } + free(recipient); + return TRUE; } @@ -1197,7 +1195,9 @@ _cmd_complete_parameters(char *input, int *size) // autocomplete nickname in chat rooms if (ui_current_win_type() == WIN_MUC) { - Autocomplete nick_ac = muc_get_roster_ac(ui_current_recipient()); + char *recipient = ui_current_recipient(); + Autocomplete nick_ac = muc_get_roster_ac(recipient); + free(recipient); if (nick_ac != NULL) { gchar *nick_choices[] = { "/msg", "/info", "/caps", "/status", "/software" } ; @@ -1979,6 +1979,8 @@ _cmd_who(gchar **args, struct cmd_help_t help) ui_room_roster(room, filtered, presence); } + free(room); + // not in groupchat window } else { cons_show(""); @@ -2143,6 +2145,8 @@ _cmd_msg(gchar **args, struct cmd_help_t help) ui_current_print_line("No such participant \"%s\" in room.", usr); } + free(room_name); + return TRUE; } else { @@ -2464,17 +2468,20 @@ _cmd_info(gchar **args, struct cmd_help_t help) jabber_conn_status_t conn_status = jabber_get_connection_status(); win_type_t win_type = ui_current_win_type(); PContact pcontact = NULL; + char *recipient; if (conn_status != JABBER_CONNECTED) { cons_show("You are not currently connected."); return TRUE; } + recipient = ui_current_recipient(); + switch (win_type) { case WIN_MUC: if (usr != NULL) { - pcontact = muc_get_participant(ui_current_recipient(), usr); + pcontact = muc_get_participant(recipient, usr); if (pcontact != NULL) { cons_show_info(pcontact); } else { @@ -2488,11 +2495,11 @@ _cmd_info(gchar **args, struct cmd_help_t help) if (usr != NULL) { cons_show("No parameter required for /info in chat."); } else { - pcontact = roster_get_contact(ui_current_recipient()); + pcontact = roster_get_contact(recipient); if (pcontact != NULL) { cons_show_info(pcontact); } else { - cons_show("No such contact \"%s\" in roster.", ui_current_recipient()); + cons_show("No such contact \"%s\" in roster.", recipient); } } break; @@ -2500,7 +2507,7 @@ _cmd_info(gchar **args, struct cmd_help_t help) if (usr != NULL) { ui_current_print_line("No parameter required when in chat."); } else { - Jid *jid = jid_create(ui_current_recipient()); + Jid *jid = jid_create(recipient); pcontact = muc_get_participant(jid->barejid, jid->resourcepart); if (pcontact != NULL) { cons_show_info(pcontact); @@ -2530,6 +2537,8 @@ _cmd_info(gchar **args, struct cmd_help_t help) break; } + free(recipient); + return TRUE; } @@ -2539,6 +2548,7 @@ _cmd_caps(gchar **args, struct cmd_help_t help) jabber_conn_status_t conn_status = jabber_get_connection_status(); win_type_t win_type = ui_current_win_type(); PContact pcontact = NULL; + char *recipient; if (conn_status != JABBER_CONNECTED) { cons_show("You are not currently connected."); @@ -2549,13 +2559,15 @@ _cmd_caps(gchar **args, struct cmd_help_t help) { case WIN_MUC: if (args[0] != NULL) { - pcontact = muc_get_participant(ui_current_recipient(), args[0]); + recipient = ui_current_recipient(); + pcontact = muc_get_participant(recipient, args[0]); if (pcontact != NULL) { Resource *resource = p_contact_get_resource(pcontact, args[0]); cons_show_caps(args[0], resource); } else { cons_show("No such participant \"%s\" in room.", args[0]); } + free(recipient); } else { cons_show("No nickname supplied to /caps in chat room."); } @@ -2588,10 +2600,15 @@ _cmd_caps(gchar **args, struct cmd_help_t help) if (args[0] != NULL) { cons_show("No parameter needed to /caps when in private chat."); } else { - Jid *jid = jid_create(ui_current_recipient()); - pcontact = muc_get_participant(jid->barejid, jid->resourcepart); - Resource *resource = p_contact_get_resource(pcontact, jid->resourcepart); - cons_show_caps(jid->resourcepart, resource); + recipient = ui_current_recipient(); + Jid *jid = jid_create(recipient); + if (jid) { + pcontact = muc_get_participant(jid->barejid, jid->resourcepart); + Resource *resource = p_contact_get_resource(pcontact, jid->resourcepart); + cons_show_caps(jid->resourcepart, resource); + jid_destroy(jid); + } + free(recipient); } break; default: @@ -2608,6 +2625,7 @@ _cmd_software(gchar **args, struct cmd_help_t help) jabber_conn_status_t conn_status = jabber_get_connection_status(); win_type_t win_type = ui_current_win_type(); PContact pcontact = NULL; + char *recipient; if (conn_status != JABBER_CONNECTED) { cons_show("You are not currently connected."); @@ -2618,14 +2636,16 @@ _cmd_software(gchar **args, struct cmd_help_t help) { case WIN_MUC: if (args[0] != NULL) { - pcontact = muc_get_participant(ui_current_recipient(), args[0]); + recipient = ui_current_recipient(); + pcontact = muc_get_participant(recipient, args[0]); if (pcontact != NULL) { - Jid *jid = jid_create_from_bare_and_resource(ui_current_recipient(), args[0]); + Jid *jid = jid_create_from_bare_and_resource(recipient, args[0]); iq_send_software_version(jid->fulljid); jid_destroy(jid); } else { cons_show("No such participant \"%s\" in room.", args[0]); } + free(recipient); } else { cons_show("No nickname supplied to /software in chat room."); } @@ -2635,11 +2655,12 @@ _cmd_software(gchar **args, struct cmd_help_t help) if (args[0] != NULL) { Jid *jid = jid_create(args[0]); - if (jid->fulljid == NULL) { + if (jid == NULL || jid->fulljid == NULL) { cons_show("You must provide a full jid to the /software command."); } else { iq_send_software_version(jid->fulljid); } + jid_destroy(jid); } else { cons_show("You must provide a jid to the /software command."); } @@ -2648,7 +2669,9 @@ _cmd_software(gchar **args, struct cmd_help_t help) if (args[0] != NULL) { cons_show("No parameter needed to /software when in private chat."); } else { - iq_send_software_version(ui_current_recipient()); + recipient = ui_current_recipient(); + iq_send_software_version(recipient); + free(recipient); } break; default: diff --git a/src/jid.c b/src/jid.c index 6347f083..81f51745 100644 --- a/src/jid.c +++ b/src/jid.c @@ -34,13 +34,14 @@ jid_create(const gchar * const str) { Jid *result = NULL; - if (str == NULL) { + /* if str is NULL g_strdup returns NULL */ + gchar *trimmed = g_strdup(str); + if (trimmed == NULL) { return NULL; } - gchar *trimmed = g_strdup(str); - if (strlen(trimmed) == 0) { + g_free(trimmed); return NULL; } @@ -50,10 +51,12 @@ jid_create(const gchar * const str) } if (!g_utf8_validate(trimmed, -1, NULL)) { + g_free(trimmed); return NULL; } result = malloc(sizeof(struct jid_t)); + result->str = NULL; result->localpart = NULL; result->domainpart = NULL; result->resourcepart = NULL; @@ -81,13 +84,11 @@ jid_create(const gchar * const str) } if (result->domainpart == NULL) { - free(trimmed); + jid_destroy(result); return NULL; } - result->str = g_strdup(trimmed); - - free(trimmed); + result->str = trimmed; return result; } @@ -181,17 +182,17 @@ char * get_room_from_full_jid(const char * const full_room_jid) { char **tokens = g_strsplit(full_room_jid, "/", 0); - char *room_part; + char *room_part = NULL; - if (tokens == NULL || tokens[0] == NULL) { - return NULL; - } else { - room_part = strdup(tokens[0]); + if (tokens != NULL) { + if (tokens[0] != NULL) { + room_part = strdup(tokens[0]); + } g_strfreev(tokens); - - return room_part; } + + return room_part; } /* @@ -203,16 +204,15 @@ char * get_nick_from_full_jid(const char * const full_room_jid) { char **tokens = g_strsplit(full_room_jid, "/", 0); - char *nick_part; + char *nick_part = NULL; - if (tokens == NULL || tokens[1] == NULL) { - return NULL; - } else { - nick_part = strdup(tokens[1]); + if (tokens != NULL) { + if (tokens[0] != NULL && tokens[1] != NULL) { + nick_part = strdup(tokens[1]); + } g_strfreev(tokens); - - return nick_part; } -} + return nick_part; +} From f2638e001aef47c04168784e6f45af7264bcd309 Mon Sep 17 00:00:00 2001 From: Dmitry Podgorny Date: Wed, 31 Jul 2013 01:53:10 +0300 Subject: [PATCH 2/3] rewritten FREE_SET_NULL and GFREE_SET_NULL NULL is valid argument for free(). Construction do {} while (0) allows using FREE_SET_NULL as one token and force to put semicolon at the end. --- src/common.h | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/common.h b/src/common.h index b87f2a97..f8ed82a5 100644 --- a/src/common.h +++ b/src/common.h @@ -42,18 +42,16 @@ #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0])) #define FREE_SET_NULL(resource) \ -{ \ - if (resource != NULL) { \ - free(resource); \ - resource = NULL; \ - } \ -} +do { \ + free(resource); \ + resource = NULL; \ +} while (0) #define GFREE_SET_NULL(resource) \ -{ \ +do { \ g_free(resource); \ resource = NULL; \ -} +} while (0) typedef enum { CONTACT_OFFLINE, From 7b37f2ace7536ae12f1726819cb0b6224232361c Mon Sep 17 00:00:00 2001 From: Dmitry Podgorny Date: Wed, 31 Jul 2013 02:03:22 +0300 Subject: [PATCH 3/3] separate glib allocator and stdlib malloc/free Memory allocated by glib should be freed by g_free. Probably g_free calls stdlib free, but in order to avoid portability issues better use g_free as described in glib documentation. --- src/jid.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/jid.c b/src/jid.c index 81f51745..b236c164 100644 --- a/src/jid.c +++ b/src/jid.c @@ -108,12 +108,12 @@ void jid_destroy(Jid *jid) { if (jid != NULL) { - FREE_SET_NULL(jid->str); - FREE_SET_NULL(jid->localpart); - FREE_SET_NULL(jid->domainpart); - FREE_SET_NULL(jid->resourcepart); - FREE_SET_NULL(jid->barejid); - FREE_SET_NULL(jid->fulljid); + GFREE_SET_NULL(jid->str); + GFREE_SET_NULL(jid->localpart); + GFREE_SET_NULL(jid->domainpart); + GFREE_SET_NULL(jid->resourcepart); + GFREE_SET_NULL(jid->barejid); + GFREE_SET_NULL(jid->fulljid); FREE_SET_NULL(jid); } }