From 750df38e8ccd59d7cb3a11e09a682755060f7283 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sun, 6 Dec 2015 22:31:49 +0100 Subject: [PATCH 1/4] Rewrite some faulty logic handling the saved channels. Issue #340 brought to our attention the fact that under certain circumstances irssi would go on a wild rampage and carelessly overwrite some saved channel records in the configuration file. This happened because the code didn't take into account the case where the channel index in setupchannels wouldn't match the one in the configuration; this actually happens when the user removes a chatnet without removing the associated channels. --- src/core/channels-setup.c | 45 ++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/src/core/channels-setup.c b/src/core/channels-setup.c index 2902ef8e..33f58022 100644 --- a/src/core/channels-setup.c +++ b/src/core/channels-setup.c @@ -30,16 +30,34 @@ GSList *setupchannels; +static int compare_channel_name (CONFIG_NODE *node, CHANNEL_SETUP_REC *channel) +{ + char *name, *chatnet; + + name = config_node_get_str(node, "name", NULL); + chatnet = config_node_get_str(node, "chatnet", NULL); + + if (name == NULL || chatnet == NULL) + return 1; + + return !!strcmp(name, channel->name) | !!strcmp(chatnet, channel->chatnet); +} + static void channel_setup_save(CHANNEL_SETUP_REC *channel) { CONFIG_NODE *parentnode, *node; - int index; - - index = g_slist_index(setupchannels, channel); + GSList *config_node; parentnode = iconfig_node_traverse("(channels", TRUE); - node = config_node_nth(parentnode, index); - if (node == NULL) + + /* Try to find this channel in the configuration */ + config_node = g_slist_find_custom(parentnode->value, channel, + (GCompareFunc)compare_channel_name); + if (config_node != NULL) + /* Let's update this channel record */ + node = config_node->data; + else + /* Create a brand-new channel record */ node = iconfig_node_section(parentnode, NULL, NODE_TYPE_BLOCK); iconfig_node_clear(node); @@ -65,10 +83,21 @@ void channel_setup_create(CHANNEL_SETUP_REC *channel) static void channel_config_remove(CHANNEL_SETUP_REC *channel) { - CONFIG_NODE *node; + CONFIG_NODE *parentnode; + GSList *config_node; - node = iconfig_node_traverse("channels", FALSE); - if (node != NULL) iconfig_node_list_remove(node, g_slist_index(setupchannels, channel)); + parentnode = iconfig_node_traverse("channels", FALSE); + + if (parentnode == NULL) + return; + + /* Try to find this channel in the configuration */ + config_node = g_slist_find_custom(parentnode->value, channel, + (GCompareFunc)compare_channel_name); + + if (config_node != NULL) + /* Delete the channel from the configuration */ + iconfig_node_remove(parentnode, config_node->data); } static void channel_setup_destroy(CHANNEL_SETUP_REC *channel) From 60c501625bbc450b8bd443a7cbccf897810634fc Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Wed, 9 Dec 2015 15:43:31 +0100 Subject: [PATCH 2/4] Better function naming --- src/core/channels-setup.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/channels-setup.c b/src/core/channels-setup.c index 33f58022..b3039fb2 100644 --- a/src/core/channels-setup.c +++ b/src/core/channels-setup.c @@ -30,7 +30,7 @@ GSList *setupchannels; -static int compare_channel_name (CONFIG_NODE *node, CHANNEL_SETUP_REC *channel) +static int compare_channel_setup (CONFIG_NODE *node, CHANNEL_SETUP_REC *channel) { char *name, *chatnet; @@ -52,7 +52,7 @@ static void channel_setup_save(CHANNEL_SETUP_REC *channel) /* Try to find this channel in the configuration */ config_node = g_slist_find_custom(parentnode->value, channel, - (GCompareFunc)compare_channel_name); + (GCompareFunc)compare_channel_setup); if (config_node != NULL) /* Let's update this channel record */ node = config_node->data; @@ -93,7 +93,7 @@ static void channel_config_remove(CHANNEL_SETUP_REC *channel) /* Try to find this channel in the configuration */ config_node = g_slist_find_custom(parentnode->value, channel, - (GCompareFunc)compare_channel_name); + (GCompareFunc)compare_channel_setup); if (config_node != NULL) /* Delete the channel from the configuration */ From 971417caa342881780b347f6aca728d962d710ee Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Wed, 9 Dec 2015 16:02:37 +0100 Subject: [PATCH 3/4] Rewrite some faulty logic handling the saved servers. --- src/core/channels-setup.c | 23 ++++++++------- src/core/servers-setup.c | 59 +++++++++++++++++++++++++++++---------- 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/src/core/channels-setup.c b/src/core/channels-setup.c index b3039fb2..dd722fea 100644 --- a/src/core/channels-setup.c +++ b/src/core/channels-setup.c @@ -40,25 +40,28 @@ static int compare_channel_setup (CONFIG_NODE *node, CHANNEL_SETUP_REC *channel) if (name == NULL || chatnet == NULL) return 1; - return !!strcmp(name, channel->name) | !!strcmp(chatnet, channel->chatnet); + if (strcmp(name, channel->name) || strcmp(chatnet, channel->chatnet)) + return 1; + + return 0; } static void channel_setup_save(CHANNEL_SETUP_REC *channel) { - CONFIG_NODE *parentnode, *node; + CONFIG_NODE *parent_node, *node; GSList *config_node; - parentnode = iconfig_node_traverse("(channels", TRUE); + parent_node = iconfig_node_traverse("(channels", TRUE); /* Try to find this channel in the configuration */ - config_node = g_slist_find_custom(parentnode->value, channel, + config_node = g_slist_find_custom(parent_node->value, channel, (GCompareFunc)compare_channel_setup); if (config_node != NULL) /* Let's update this channel record */ node = config_node->data; else /* Create a brand-new channel record */ - node = iconfig_node_section(parentnode, NULL, NODE_TYPE_BLOCK); + node = iconfig_node_section(parent_node, NULL, NODE_TYPE_BLOCK); iconfig_node_clear(node); iconfig_node_set_str(node, "name", channel->name); @@ -83,21 +86,21 @@ void channel_setup_create(CHANNEL_SETUP_REC *channel) static void channel_config_remove(CHANNEL_SETUP_REC *channel) { - CONFIG_NODE *parentnode; + CONFIG_NODE *parent_node; GSList *config_node; - parentnode = iconfig_node_traverse("channels", FALSE); + parent_node = iconfig_node_traverse("channels", FALSE); - if (parentnode == NULL) + if (parent_node == NULL) return; /* Try to find this channel in the configuration */ - config_node = g_slist_find_custom(parentnode->value, channel, + config_node = g_slist_find_custom(parent_node->value, channel, (GCompareFunc)compare_channel_setup); if (config_node != NULL) /* Delete the channel from the configuration */ - iconfig_node_remove(parentnode, config_node->data); + iconfig_node_remove(parent_node, config_node->data); } static void channel_setup_destroy(CHANNEL_SETUP_REC *channel) diff --git a/src/core/servers-setup.c b/src/core/servers-setup.c index 771e3999..26632d63 100644 --- a/src/core/servers-setup.c +++ b/src/core/servers-setup.c @@ -423,17 +423,41 @@ static SERVER_SETUP_REC *server_setup_read(CONFIG_NODE *node) return rec; } +static int compare_server_setup (CONFIG_NODE *node, SERVER_SETUP_REC *server) +{ + char *address, *chatnet; + int port; + + address = config_node_get_str(node, "address", NULL); + chatnet = config_node_get_str(node, "chatnet", NULL); + port = config_node_get_int(node, "port", 0); + + if (address == NULL || chatnet == NULL) + return 1; + + if (strcmp(address, server->address) || strcmp(chatnet, server->chatnet) + || port != server->port) + return 1; + + return 0; +} + static void server_setup_save(SERVER_SETUP_REC *rec) { - CONFIG_NODE *parentnode, *node; - int index; + CONFIG_NODE *parent_node, *node; + GSList *config_node; - index = g_slist_index(setupservers, rec); + parent_node = iconfig_node_traverse("(servers", TRUE); - parentnode = iconfig_node_traverse("(servers", TRUE); - node = config_node_nth(parentnode, index); - if (node == NULL) - node = iconfig_node_section(parentnode, NULL, NODE_TYPE_BLOCK); + /* Try to find this channel in the configuration */ + config_node = g_slist_find_custom(parent_node->value, rec, + (GCompareFunc)compare_server_setup); + if (config_node != NULL) + /* Let's update this server record */ + node = config_node->data; + else + /* Create a brand-new server record */ + node = iconfig_node_section(parent_node, NULL, NODE_TYPE_BLOCK); iconfig_node_clear(node); iconfig_node_set_str(node, "address", rec->address); @@ -465,14 +489,21 @@ static void server_setup_save(SERVER_SETUP_REC *rec) static void server_setup_remove_config(SERVER_SETUP_REC *rec) { - CONFIG_NODE *node; - int index; + CONFIG_NODE *parent_node; + GSList *config_node; - node = iconfig_node_traverse("servers", FALSE); - if (node != NULL) { - index = g_slist_index(setupservers, rec); - iconfig_node_list_remove(node, index); - } + parent_node = iconfig_node_traverse("servers", FALSE); + + if (parent_node == NULL) + return; + + /* Try to find this server in the configuration */ + config_node = g_slist_find_custom(parent_node->value, rec, + (GCompareFunc)compare_server_setup); + + if (config_node != NULL) + /* Delete the server from the configuration */ + iconfig_node_remove(parent_node, config_node->data); } static void server_setup_destroy(SERVER_SETUP_REC *rec) From 1749a7a5abf9801bb663b74d4b3e7c3bcbfeb271 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Wed, 9 Dec 2015 16:16:03 +0100 Subject: [PATCH 4/4] Minor adjustments. Use g_strcmp0 instead of strcmp. Explicit checks added for the g_strcmp0 clauses. --- src/core/channels-setup.c | 6 ++---- src/core/servers-setup.c | 8 +++----- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/core/channels-setup.c b/src/core/channels-setup.c index dd722fea..4966d77d 100644 --- a/src/core/channels-setup.c +++ b/src/core/channels-setup.c @@ -37,10 +37,8 @@ static int compare_channel_setup (CONFIG_NODE *node, CHANNEL_SETUP_REC *channel) name = config_node_get_str(node, "name", NULL); chatnet = config_node_get_str(node, "chatnet", NULL); - if (name == NULL || chatnet == NULL) - return 1; - - if (strcmp(name, channel->name) || strcmp(chatnet, channel->chatnet)) + if (g_strcmp0(name, channel->name) != 0 || + g_strcmp0(chatnet, channel->chatnet) != 0) return 1; return 0; diff --git a/src/core/servers-setup.c b/src/core/servers-setup.c index 26632d63..4a048282 100644 --- a/src/core/servers-setup.c +++ b/src/core/servers-setup.c @@ -432,11 +432,9 @@ static int compare_server_setup (CONFIG_NODE *node, SERVER_SETUP_REC *server) chatnet = config_node_get_str(node, "chatnet", NULL); port = config_node_get_int(node, "port", 0); - if (address == NULL || chatnet == NULL) - return 1; - - if (strcmp(address, server->address) || strcmp(chatnet, server->chatnet) - || port != server->port) + if (g_strcmp0(address, server->address) != 0 || + g_strcmp0(chatnet, server->chatnet) != 0 || + port != server->port) return 1; return 0;