From dceac3e4fb2f3cf7900089ab6d14b91c5670a585 Mon Sep 17 00:00:00 2001 From: Keith Buck Date: Fri, 28 Feb 2014 07:02:49 +0000 Subject: [PATCH] conf parsing: Fix memory leaks and clean up code a bit. Charybdis currently leaks about 45-50k per configuration parse, including every rehash. This change plugs these leaks by properly iterating through all conf_parm_t structures to seek all strings that should be freed and also by freeing the conf_parm_t structures themselves. These leaks have been present since the original rewrite of the configuration parsing system in ircd-ratbox r11953. Additionally, this change also cleans up and documents the parsing code a bit. --- include/newconf.h | 24 ++++++++++++++++-------- src/ircd_parser.y | 29 ++++++++++++++--------------- src/newconf.c | 14 +++++++------- 3 files changed, 37 insertions(+), 30 deletions(-) diff --git a/include/newconf.h b/include/newconf.h index 53d3371b..fafad9e3 100644 --- a/include/newconf.h +++ b/include/newconf.h @@ -25,19 +25,27 @@ struct TopConf }; -#define CF_QSTRING 0x01 +#define CF_QSTRING 0x01 /* quoted string */ #define CF_INT 0x02 -#define CF_STRING 0x03 +#define CF_STRING 0x03 /* unquoted string */ #define CF_TIME 0x04 #define CF_YESNO 0x05 -#define CF_LIST 0x06 -#define CF_ONE 0x07 -#define CF_MTYPE 0xFF +#define CF_MTYPE 0xFF /* mask for type */ -#define CF_FLIST 0x1000 -#define CF_MFLAG 0xFF00 +/* CF_FLIST is used to allow specifying that an option accepts a list of (type) + * values. conf_parm_t.type will never actually have another type & CF_FLIST; + * it's only used as a true flag in newconf.c (which only consumes conf_parm_t + * structures and doesn't create them itself). + */ +#define CF_FLIST 0x0100 /* flag for list */ +#define CF_MFLAG 0xFF00 /* mask for flags */ +/* conf_parm_t.type must be either one type OR one flag. this is pretty easy to + * enforce because lists always contain nested conf_parm_t structures whose + * .type is the real type, so it doesn't need to be stored in the top-level one + * anyway. + */ typedef struct conf_parm_t_stru { struct conf_parm_t_stru *next; @@ -59,7 +67,7 @@ extern char *current_file; int read_config(char *); int conf_start_block(char *, char *); int conf_end_block(struct TopConf *); -int conf_call_set(struct TopConf *, char *, conf_parm_t *, int); +int conf_call_set(struct TopConf *, char *, conf_parm_t *); void conf_report_error(const char *, ...); void newconf_init(void); int add_conf_item(const char *topconf, const char *name, int type, void (*func) (void *)); diff --git a/src/ircd_parser.y b/src/ircd_parser.y index 40d4523d..aeb9bdd1 100644 --- a/src/ircd_parser.y +++ b/src/ircd_parser.y @@ -101,20 +101,20 @@ static int conf_get_yesno_value(char *str) static void free_cur_list(conf_parm_t* list) { - switch (list->type & CF_MTYPE) - { - case CF_STRING: - case CF_QSTRING: - rb_free(list->v.string); - break; - case CF_LIST: - free_cur_list(list->v.list); - break; - default: break; + if (list->type == CF_STRING || list->type == CF_QSTRING) { + rb_free(list->v.string); + } else if (list->type == CF_FLIST) { + /* Even though CF_FLIST is a flag, comparing with == is valid + * because conf_parm_t.type must be either a type or one flag. + */ + free_cur_list(list->v.list); } - if (list->next) + if (list->next) { free_cur_list(list->next); + } + + rb_free(list); } @@ -125,7 +125,7 @@ static void add_cur_list_cpt(conf_parm_t *new) if (cur_list == NULL) { cur_list = rb_malloc(sizeof(conf_parm_t)); - cur_list->type |= CF_FLIST; + cur_list->type = CF_FLIST; cur_list->v.list = new; } else @@ -216,7 +216,7 @@ block_items: block_items block_item block_item: string '=' itemlist ';' { - conf_call_set(conf_cur_block, $1, cur_list, CF_LIST); + conf_call_set(conf_cur_block, $1, cur_list); free_cur_list(cur_list); cur_list = NULL; } @@ -233,8 +233,7 @@ single: oneitem | oneitem TWODOTS oneitem { /* "1 .. 5" meaning 1,2,3,4,5 - only valid for integers */ - if (($1->type & CF_MTYPE) != CF_INT || - ($3->type & CF_MTYPE) != CF_INT) + if ($1->type != CF_INT || $3->type != CF_INT) { conf_report_error("Both arguments in '..' notation must be integers."); break; diff --git a/src/newconf.c b/src/newconf.c index 2ff8c1ef..03206f58 100644 --- a/src/newconf.c +++ b/src/newconf.c @@ -65,7 +65,7 @@ static char *yy_privset_extends = NULL; static const char * conf_strtype(int type) { - switch (type & CF_MTYPE) + switch (CF_TYPE(type)) { case CF_INT: return "integer value"; @@ -424,7 +424,7 @@ set_modes_from_table(int *modes, const char *whatis, struct mode_table *tab, con int dir = 1; int mode; - if((args->type & CF_MTYPE) != CF_STRING) + if(CF_TYPE(args->type) != CF_STRING) { conf_report_error("Warning -- %s is not a string; ignoring.", whatis); continue; @@ -857,7 +857,7 @@ conf_set_listen_port_both(void *data, int ssl) conf_parm_t *args = data; for (; args; args = args->next) { - if((args->type & CF_MTYPE) != CF_INT) + if(CF_TYPE(args->type) != CF_INT) { conf_report_error ("listener::port argument is not an integer " "-- ignoring."); @@ -1179,7 +1179,7 @@ conf_set_shared_oper(void *data) if(args->next != NULL) { - if((args->type & CF_MTYPE) != CF_QSTRING) + if(CF_TYPE(args->type) != CF_QSTRING) { conf_report_error("Ignoring shared::oper -- server is not a qstring"); return; @@ -1191,7 +1191,7 @@ conf_set_shared_oper(void *data) else yy_shared->server = rb_strdup("*"); - if((args->type & CF_MTYPE) != CF_QSTRING) + if(CF_TYPE(args->type) != CF_QSTRING) { conf_report_error("Ignoring shared::oper -- oper is not a qstring"); return; @@ -2037,7 +2037,7 @@ conf_set_generic_string(void *data, int len, void *location) } int -conf_call_set(struct TopConf *tc, char *item, conf_parm_t * value, int type) +conf_call_set(struct TopConf *tc, char *item, conf_parm_t * value) { struct ConfEntry *cf; conf_parm_t *cp; @@ -2437,7 +2437,7 @@ newconf_init() add_top_conf("auth", conf_begin_auth, conf_end_auth, conf_auth_table); add_top_conf("shared", conf_cleanup_shared, conf_cleanup_shared, NULL); - add_conf_item("shared", "oper", CF_QSTRING|CF_FLIST, conf_set_shared_oper); + add_conf_item("shared", "oper", CF_QSTRING | CF_FLIST, conf_set_shared_oper); add_conf_item("shared", "flags", CF_STRING | CF_FLIST, conf_set_shared_flags); add_top_conf("connect", conf_begin_connect, conf_end_connect, conf_connect_table);