From afb8f1030615a15a9e26db49a6864c4b2ee4a280 Mon Sep 17 00:00:00 2001 From: Per Cederqvist Date: Tue, 3 Jan 2006 13:18:25 +0000 Subject: [PATCH] Fixed various harmless memory and file descriptor leaks found by valgrind. * src/server/testsuite/lyskomd.supp: Added aid_lexer_2. * src/server/testsuite/lyskomd.0/regexp-match-cov.exp (shutdown): Don't expect any suppressed leaked blocks. * src/server/testsuite/lyskomd.0/aux-items-cov.exp: Expect less suppressed leaked blocks. * src/server/testsuite/lyskomd.0/37.exp: Adjusted to new report format when configuration errors are found. * src/server/testsuite/lyskomd.0/conf-file-cov.exp: Ditto. * src/server/simple-cache.c (free_all_cache): Close text_file and file_a. * src/server/testsuite/lyskomd.supp: Removed i_fopen-2.3.1 and i_fopen-2.3.5, which were actually caused by the above problem. * src/server/server-config.c (read_configuration): Check the return value of read_config(). Free the configuration before calling restart_kom if there are any problems with the configuration. Report the name of the configuation file if problems are found. * src/server/ramkomd.c (dump_exit_statistics): Call clear_info() instead of free_kom_info. (free_kom_info): Removed. The new clear_info() function contains a better implementation, with less code duplication. * src/server/memory.c, src/server/kom-memory.h (clear_info): New function. * src/server/dbck.c (free_person_scratchpad): New static function. (free_person_scratch): Ditto. (main): Call free_person_scratch(), clear_info(), free_configuration() and free_all_dbck_cache() when terminating to make it possible to check for leaks with valgrind. * src/server/dbck-cache.c, src/server/dbck-cache.h (free_all_dbck_cache): New function. * src/server/conf-file.c, src/server/conf-file.h (read_config): Changed return type from void to Success. Return FAILURE instead of calling restart_kom when configuration errors are found. --- ChangeLog | 38 +++++++++++++++++++ src/server/conf-file.c | 11 +++--- src/server/conf-file.h | 2 +- src/server/dbck-cache.c | 30 +++++++++++++++ src/server/dbck-cache.h | 1 + src/server/dbck.c | 26 +++++++++++++ src/server/kom-memory.h | 3 ++ src/server/memory.c | 9 +++++ src/server/ramkomd.c | 20 +--------- src/server/server-config.c | 9 ++++- src/server/simple-cache.c | 3 ++ src/server/testsuite/lyskomd.0/37.exp | 4 +- .../testsuite/lyskomd.0/aux-items-cov.exp | 2 +- .../testsuite/lyskomd.0/conf-file-cov.exp | 4 +- .../testsuite/lyskomd.0/regexp-match-cov.exp | 2 +- src/server/testsuite/lyskomd.supp | 38 ++++++------------- 16 files changed, 144 insertions(+), 58 deletions(-) diff --git a/ChangeLog b/ChangeLog index 539b2a8f..ee91c922 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,41 @@ +2006-01-03 Per Cederqvist + + Fixed various harmless memory and file descriptor leaks found by + valgrind. + * src/server/testsuite/lyskomd.supp: Added aid_lexer_2. + * src/server/testsuite/lyskomd.0/regexp-match-cov.exp (shutdown): + Don't expect any suppressed leaked blocks. + * src/server/testsuite/lyskomd.0/aux-items-cov.exp: Expect less + suppressed leaked blocks. + * src/server/testsuite/lyskomd.0/37.exp: Adjusted to new report + format when configuration errors are found. + * src/server/testsuite/lyskomd.0/conf-file-cov.exp: Ditto. + * src/server/simple-cache.c (free_all_cache): Close text_file and + file_a. + * src/server/testsuite/lyskomd.supp: Removed i_fopen-2.3.1 and + i_fopen-2.3.5, which were actually caused by the above problem. + * src/server/server-config.c (read_configuration): Check the + return value of read_config(). Free the configuration before + calling restart_kom if there are any problems with the + configuration. Report the name of the configuation file if + problems are found. + * src/server/ramkomd.c (dump_exit_statistics): Call clear_info() + instead of free_kom_info. + (free_kom_info): Removed. The new clear_info() function contains + a better implementation, with less code duplication. + * src/server/memory.c, src/server/kom-memory.h (clear_info): New + function. + * src/server/dbck.c (free_person_scratchpad): New static function. + (free_person_scratch): Ditto. + (main): Call free_person_scratch(), clear_info(), + free_configuration() and free_all_dbck_cache() when terminating to + make it possible to check for leaks with valgrind. + * src/server/dbck-cache.c, src/server/dbck-cache.h + (free_all_dbck_cache): New function. + * src/server/conf-file.c, src/server/conf-file.h (read_config): + Changed return type from void to Success. Return FAILURE instead + of calling restart_kom when configuration errors are found. + 2006-01-02 Per Cederqvist Update valgrind options to valgrind-2.4.0. diff --git a/src/server/conf-file.c b/src/server/conf-file.c index 0c0ff830..b941a4fe 100644 --- a/src/server/conf-file.c +++ b/src/server/conf-file.c @@ -213,7 +213,7 @@ configure_line(FILE *fp, return 0; } -void +Success read_config(const char *config_file, const struct parameter *par) { @@ -231,12 +231,13 @@ read_config(const char *config_file, i_fclose(fp); assign_defaults(par, &errs); - if (errs) - restart_kom("Please fix the above errors in %s and restart lyskomd\n", - config_file); - sfree(assignment_count); assignment_count = 0; + + if (errs) + return FAILURE; + else + return OK; } Success diff --git a/src/server/conf-file.h b/src/server/conf-file.h index 923210a6..e3fbb569 100644 --- a/src/server/conf-file.h +++ b/src/server/conf-file.h @@ -41,7 +41,7 @@ struct parameter { cares about such things. */ }; -void read_config(const char *config_file, const struct parameter *par); +Success read_config(const char *config_file, const struct parameter *par); extern Success assign_text_no (const char *val, const struct parameter *par); extern Success assign_conf_no (const char *val, const struct parameter *par); diff --git a/src/server/dbck-cache.c b/src/server/dbck-cache.c index f2b8f4df..7d725802 100644 --- a/src/server/dbck-cache.c +++ b/src/server/dbck-cache.c @@ -1311,3 +1311,33 @@ init_cache(void) return OK; } + +void +free_all_dbck_cache(void) +{ + unsigned int i; + + for ( i = 1; i < next_free_num; i++ ) + { + if (pers_arr[i] != NULL) + free_person(pers_arr[i]); + if (conf_arr[i] != NULL) + { + free_conference(conf_arr[i]); + s_clear(&name_list[i]); + } + } + + for ( i = 1; i < next_text_num; i++ ) + if (text_arr[i] != NULL) + free_text_stat(text_arr[i]); + + sfree(pers_arr); + sfree(conf_arr); + sfree(text_arr); + sfree(name_list); + + i_fclose(text_file); + if (new_text_file != NULL) + i_fclose(new_text_file); +} diff --git a/src/server/dbck-cache.h b/src/server/dbck-cache.h index 643b0fbb..4d3d5431 100644 --- a/src/server/dbck-cache.h +++ b/src/server/dbck-cache.h @@ -33,5 +33,6 @@ cached_flush_text(Text_no text_no, extern void cache_open_new_text_file(void); +extern void free_all_dbck_cache(void); extern long oformat; diff --git a/src/server/dbck.c b/src/server/dbck.c index b818be1f..bb57f069 100644 --- a/src/server/dbck.c +++ b/src/server/dbck.c @@ -81,6 +81,7 @@ #include "lockdb.h" #include "linkansi.h" #include "eintr.h" +#include "kom-memory.h" #define OPT_PERS_PRESENTATION_CONF 1 @@ -206,6 +207,13 @@ alloc_person_scratchpad(void) return p; } +static void +free_person_scratchpad(Person_scratchpad *p) +{ + if (p != NULL) + sfree(p); +} + static Bool is_comment_to(Text_no comment, @@ -1248,6 +1256,19 @@ init_person_scratch(void) } } +static void +free_person_scratch(void) +{ + Pers_no i; + + if (person_scratchpad != NULL) + { + for (i = 0; i < param.max_conf; i++) + free_person_scratchpad(person_scratchpad[i]); + sfree(person_scratchpad); + } +} + static long post_check_persons(void) { @@ -1682,6 +1703,11 @@ main (int argc, if (have_lock) unlock_db(); + free_person_scratch(); + clear_info(&kom_info); + free_configuration(); + free_all_dbck_cache(); + /* Don't actually die until something is entered on stdin in debug mode. This is mainly here for the benefit of the test suite, but is could also be useful to be able to attach a debugger and diff --git a/src/server/kom-memory.h b/src/server/kom-memory.h index 2eaccaa3..0ab2f71a 100644 --- a/src/server/kom-memory.h +++ b/src/server/kom-memory.h @@ -116,6 +116,9 @@ extern void init_who_info_ident (Who_info_ident *w); /* Who_info_old */ extern void init_who_info_old (Who_info_old *w); +/* Kom_info */ +extern void clear_info (Info *i); + /* Aux_item_list */ extern void free_aux_item_list (Aux_item_list *list); diff --git a/src/server/memory.c b/src/server/memory.c index 7e8c55d5..6c8f5aee 100644 --- a/src/server/memory.c +++ b/src/server/memory.c @@ -688,6 +688,15 @@ init_who_info_old (Who_info_old *w) w->working_conference = 0; } +/* Kom_info */ + +void +clear_info(Info *i) +{ + free_aux_item_list(&i->aux_item_list); +} + + /* Aux_item_list */ void diff --git a/src/server/ramkomd.c b/src/server/ramkomd.c index 08d16584..c8a88138 100644 --- a/src/server/ramkomd.c +++ b/src/server/ramkomd.c @@ -132,7 +132,6 @@ static int foreground = 0; static oop_adapter_signal *kom_signal_adapter; static void dump_exit_statistics(void); -static void free_kom_info(void); static oop_call_signal sighandler_term; static oop_call_signal sighandler_quit; static oop_call_signal sighandler_usr1; @@ -701,7 +700,7 @@ dump_exit_statistics(void) free_all_tmp(); free_all_cache(); free_all_jubel(); - free_kom_info(); + clear_info(&kom_info); free_aux_item_definitions(); free_configuration(); free_default_config_file_name(); @@ -716,20 +715,3 @@ dump_exit_statistics(void) dump_l2g_stats(stat_file); i_fclose (stat_file); } - -static void -free_kom_info(void) -{ - unsigned long i; - - if (kom_info.aux_item_list.items != NULL) - { - for (i = 0; i < kom_info.aux_item_list.length; i++) - { - s_clear(&kom_info.aux_item_list.items[i].data); - } - sfree(kom_info.aux_item_list.items); - } - kom_info.aux_item_list.length = 0; - kom_info.aux_item_list.items = 0; -} diff --git a/src/server/server-config.c b/src/server/server-config.c index 951ed848..9c0a51df 100644 --- a/src/server/server-config.c +++ b/src/server/server-config.c @@ -754,7 +754,8 @@ read_configuration(const char *conf_file) { Bool err = FALSE; - read_config(conf_file, parameters); + if (read_config(conf_file, parameters) != OK) + err = TRUE; assert(param.dbase_dir != NULL); assert(param.datafile_name != NULL); @@ -917,7 +918,11 @@ read_configuration(const char *conf_file) There are probably many more things to check. */ if (err) - restart_kom("Please fix the above configuration errors.\n"); + { + free_configuration(); + restart_kom("Please fix the above configuration errors in %s.\n", + conf_file); + } } void diff --git a/src/server/simple-cache.c b/src/server/simple-cache.c index f0d73cdc..c66d9971 100644 --- a/src/server/simple-cache.c +++ b/src/server/simple-cache.c @@ -3040,6 +3040,9 @@ free_all_cache (void) sfree(small_conf_arr); sfree (match_table); + i_fclose(text_file); + if (file_a != NULL) + i_fclose(file_a); } diff --git a/src/server/testsuite/lyskomd.0/37.exp b/src/server/testsuite/lyskomd.0/37.exp index bacc5f6c..a75f1032 100644 --- a/src/server/testsuite/lyskomd.0/37.exp +++ b/src/server/testsuite/lyskomd.0/37.exp @@ -198,7 +198,7 @@ lyskomd_fail_start \ "assigner for Garb interval failed" \ "Overflow for parameter Sync retry interval \\($any_float us\\)" \ "assigner for Sync retry interval failed" \ - "Please fix the above errors in config/lyskomd-config and restart lyskomd" \ + "Please fix the above configuration errors in config/lyskomd-config." \ ] "" [exec ./timeval-overflow] lyskomd_fail_start { @@ -206,6 +206,6 @@ lyskomd_fail_start { "assigner for Garb interval failed" "Negative values not supported for parameter Sync timeout" "assigner for Sync timeout failed" - "Please fix the above errors in config/lyskomd-config and restart lyskomd" + "Please fix the above configuration errors in config/lyskomd-config." } "" "Garb interval: 4 lifetimes Sync timeout: -2 us" diff --git a/src/server/testsuite/lyskomd.0/aux-items-cov.exp b/src/server/testsuite/lyskomd.0/aux-items-cov.exp index ea1a2677..ebda8071 100644 --- a/src/server/testsuite/lyskomd.0/aux-items-cov.exp +++ b/src/server/testsuite/lyskomd.0/aux-items-cov.exp @@ -444,7 +444,7 @@ kom_enable 255 send "9999 44 0\n" simple_expect "=9999" client_death 0 -lyskomd_death {"Bug 689" 1 0 0 25} +lyskomd_death {"Bug 689" 1 0 0 2} # Try some broken config files diff --git a/src/server/testsuite/lyskomd.0/conf-file-cov.exp b/src/server/testsuite/lyskomd.0/conf-file-cov.exp index e0bc73ba..bb05c460 100644 --- a/src/server/testsuite/lyskomd.0/conf-file-cov.exp +++ b/src/server/testsuite/lyskomd.0/conf-file-cov.exp @@ -87,7 +87,9 @@ lyskomd_fail_start { "assigner for Message of the day failed" "assigner for Garb interval failed" "Parameter Max conferences only assigned 0 times \\(1 times needed\\)" - "Please fix the above errors in config/lyskomd-config and restart lyskomd" + "Parameter 'Max texts' must be at least 1." + "Parameter 'Max conferences' must be at least 5." + "Please fix the above configuration errors in config/lyskomd-config." } "" "" $conf_file lyskomd_fail_start { diff --git a/src/server/testsuite/lyskomd.0/regexp-match-cov.exp b/src/server/testsuite/lyskomd.0/regexp-match-cov.exp index bb198c8d..07887b77 100644 --- a/src/server/testsuite/lyskomd.0/regexp-match-cov.exp +++ b/src/server/testsuite/lyskomd.0/regexp-match-cov.exp @@ -98,4 +98,4 @@ simple_expect "=2003 4 { [holl "QERSON 6"] 1001 6 [holl "QersoN 8"] 1001 8 [holl send "3001 74 [holl "Error\["] 0 0\n" simple_expect "%3001 43 0" -shutdown {"Bug 689" 0 0 0 26} +shutdown diff --git a/src/server/testsuite/lyskomd.supp b/src/server/testsuite/lyskomd.supp index 73ab929d..b001ac08 100644 --- a/src/server/testsuite/lyskomd.supp +++ b/src/server/testsuite/lyskomd.supp @@ -187,47 +187,33 @@ ##----------------------------------------------------------------------## -# fopen() under glibc-2.3.1 apparently triggers a leaks. It seems to -# happen only once, during initialization, and is probably no real -# leak at all. +# flex-2.5.4a seems to not delete the current buffer once it is done. +# This only leaks a single block, however. { - i_fopen-2.3.1 + aid_lexer Memcheck:Leak fun:malloc - fun:__fopen_internal - fun:_IO_fopen@@GLIBC_2.1 - fun:i_fopen - fun:init_cache -} - -# Ditto under glibc-2.3.5. -{ - i_fopen-2.3.5 - Memcheck:Leak - fun:malloc - obj:libc-2.3.5.so - fun:fopen - fun:i_fopen - fun:post_sync + fun:yy_flex_alloc + fun:yy_create_buffer + fun:aid_lex + fun:aid_parse + fun:parse_aux_item_definitions + fun:initialize_aux_items + fun:initialize } -##----------------------------------------------------------------------## - -# flex-2.5.4a seems to not delete the current buffer once it is done. -# This only leaks a single block, however. - { - aid_lexer + aid_lexer_2 Memcheck:Leak fun:malloc - fun:yy_flex_alloc fun:yy_create_buffer fun:aid_lex fun:aid_parse fun:parse_aux_item_definitions fun:initialize_aux_items fun:initialize + fun:main } ##----------------------------------------------------------------------## -- GitLab