From dc65390c37bd67221aa6822f21f0217cfc24b3e8 Mon Sep 17 00:00:00 2001 From: Per Cederqvist Date: Wed, 28 Dec 2005 15:42:44 +0000 Subject: [PATCH] Cleanup of simple-cache.c. (Bug 172). Allocate less memory when performing a pattern name match. * src/server/simple-cache.c: Removed obsolete comments. Fixed spelling errors in comments. (find_matching_info_compare): Don't overflow if a Conf_no doesn't fit in an int. (find_matching_info): Removed obsolete comments (bug 172). (rebuild_matching_info_entry): Added a comment that explains a magic "2" in the code. (build_matching_info): Use cached_no_of_existing_conferences() instead of next_free_num when deciding how large temporary table we need. Reduce the number of loop induction variables. (cached_lookup_name): Made code more readable by introducing a temporary variable. --- ChangeLog | 15 +++++++++++ src/server/simple-cache.c | 52 ++++++++++++++++++++------------------- 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/ChangeLog b/ChangeLog index d77f2578..3c44cd49 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,20 @@ 2005-12-28 Per Cederqvist + Cleanup of simple-cache.c. (Bug 172). Allocate less memory when + performing a pattern name match. + * src/server/simple-cache.c: Removed obsolete comments. Fixed + spelling errors in comments. + (find_matching_info_compare): Don't overflow if a Conf_no doesn't + fit in an int. + (find_matching_info): Removed obsolete comments (bug 172). + (rebuild_matching_info_entry): Added a comment that explains a + magic "2" in the code. + (build_matching_info): Use cached_no_of_existing_conferences() + instead of next_free_num when deciding how large temporary table + we need. Reduce the number of loop induction variables. + (cached_lookup_name): Made code more readable by introducing a + temporary variable. + Improved test suite logging. * src/server/testsuite/config/unix.exp (simple_expect): Log what we are looking for and when we are done looking at verbosity level diff --git a/src/server/simple-cache.c b/src/server/simple-cache.c index 750700c0..f0d73cdc 100644 --- a/src/server/simple-cache.c +++ b/src/server/simple-cache.c @@ -115,10 +115,6 @@ static Conf_no existing_confs = 0; static Cache_node_mcb * text_mcb; static Text_no next_text_num = 1; -/* - * The elements in the following lists with same index refers to the same - * conference. - */ static int no_of_match_info; EXPORT Matching_info *match_table = NULL; @@ -1449,7 +1445,7 @@ free_match_table(Matching_info *mtch) /* Find the entry for CONF_NO in match_table. Return something if there is a nonempty name or a list of tokens in the entry. The - caller is responsible to checking that the name of the returned + caller is responsible for checking that the name of the returned entry is nonempty. The reason for this is that cached_delete_conf clears the name before rebuilding the matching_info, and rebuild_matching_info_entry needs to get the entry to free the @@ -1462,7 +1458,15 @@ find_matching_info_compare(const void *a, const void *b) Matching_info *info_a = (Matching_info *)a; Matching_info *info_b = (Matching_info *)b; - return info_a->conf_no - info_b->conf_no; + /* A simple subtraction could be used, but this is more + future-proof if we have to increase the size of Conf_no in the + future. */ + if (info_a->conf_no < info_b->conf_no) + return -1; + else if (info_a->conf_no > info_b->conf_no) + return 1; + else + return 0; } static Matching_info * @@ -1470,15 +1474,6 @@ find_matching_info(Conf_no conf_no) { Matching_info *info, key; - /* FIXME (bug 172): The comment below, the fact that bsearch() is - used, and the comment above find_matching_info_compare() seems - to indicate that cleanup is required here. */ - - /* FIXME: Do a binary search here! The search must be able to - FIXME: deal with NULL entries. We could do this with a regular - FIXME: bearch and a final check that the entry has a non-NULL - FIXME: tokens field. */ - key.conf_no = conf_no; info = bsearch(&key, match_table, @@ -1582,6 +1577,7 @@ rebuild_matching_info_entry(Conf_no conf_no) /* Add an entry to the match table */ + /* We need space for the new entry and for the sentinel. */ match_table = srealloc(match_table, (no_of_match_info + 2) * sizeof(Matching_info)); match_table[no_of_match_info].name = small_conf_arr[conf_no]->name; @@ -1605,17 +1601,18 @@ build_matching_info(void) { Conf_no i; Matching_info *mtch; - Conf_no *conf; + Conf_no size; free_match_table(match_table); - - match_table = srealloc(match_table, next_free_num * sizeof(Matching_info)); + + size = cached_no_of_existing_conferences() + 1; + match_table = srealloc(match_table, size * sizeof(Matching_info)); no_of_match_info = 0; mtch = match_table; - for ( i = 1; i < next_free_num; i++ ) + for ( i = 1; i < next_free_num && no_of_match_info < size; i++ ) { if ( small_conf_arr[ i ] != NULL && ! s_empty ( small_conf_arr[ i ]->name ) ) @@ -1624,10 +1621,13 @@ build_matching_info(void) mtch->tokens = tokenize(mtch->name, s_fcrea_str(WHITESPACE)); mtch->conf_no = i; ++mtch; - ++conf; ++no_of_match_info; } } + if (no_of_match_info >= size) + restart_kom("Internal error: build_matching_info: " + "no_of_match_info=%ld but size=%ld\n", + (long)no_of_match_info, (long)size); mtch->name = EMPTY_STRING; mtch->tokens = NULL; @@ -1645,6 +1645,7 @@ cached_lookup_name(const String name, { Parse_info tmp; int i; + Conf_no cno_tmp; /* FIXME (bug 174): It is a waste of resources to include persons here if this lookup is performed due to a lookup-z-name that @@ -1672,9 +1673,10 @@ cached_lookup_name(const String name, { if (s_empty(match_table[i].name)) continue; - result->conf_nos[result->no_of_conf_nos] = match_table[i].conf_no; + cno_tmp = match_table[i].conf_no; + result->conf_nos[result->no_of_conf_nos] = cno_tmp; result->type_of_conf[result->no_of_conf_nos] = - small_conf_arr[match_table[i].conf_no]->type; + small_conf_arr[cno_tmp]->type; result->no_of_conf_nos++; } } @@ -1688,9 +1690,9 @@ cached_lookup_name(const String name, for ( i = 0; i < tmp.no_of_matches; i++ ) { - result->conf_nos[ i ] = match_table[ tmp.indexes[ i ] ].conf_no; - result->type_of_conf[ i ] - = small_conf_arr[ match_table[ tmp.indexes[ i ] ].conf_no ]->type; + cno_tmp = match_table[ tmp.indexes[i] ].conf_no; + result->conf_nos[i] = cno_tmp; + result->type_of_conf[i] = small_conf_arr[cno_tmp]->type; } } sfree(tmp.indexes); -- GitLab