From 67583e736a008a6106a0f4297a0e0df6a9b90c21 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Fri, 23 Aug 2024 21:31:07 +0100 Subject: [PATCH] Move from table to tree for lookups --- src/src/acl.c | 6 +- src/src/drtables.c | 126 +++++++++++++--------------- src/src/exim.c | 55 +++++++------ src/src/expand.c | 42 ++++++---- src/src/functions.h | 14 ++-- src/src/globals.h | 4 +- src/src/lookupapi.h | 1 + src/src/lookups/ldap.c | 2 +- src/src/macros.h | 2 +- src/src/match.c | 12 +-- src/src/search.c | 182 ++++++++++++++++++++--------------------- src/src/spool_in.c | 6 +- src/src/spool_out.c | 6 +- src/src/store.c | 17 +++- src/src/structs.h | 2 +- src/src/tree.c | 6 +- src/src/verify.c | 25 +++--- test/runtest | 4 +- test/stderr/2610 | 8 +- test/stderr/2620 | 4 +- 20 files changed, 270 insertions(+), 254 deletions(-) diff --git a/src/src/acl.c b/src/src/acl.c index 69777cf9c..3f1ff65cb 100644 --- a/src/src/acl.c +++ b/src/src/acl.c @@ -5017,7 +5017,11 @@ if (is_tainted(value)) { int q = quoter_for_address(value); putc('-', f); - if (is_real_quoter(q)) fprintf(f, "(%s)", lookup_list[q]->name); + if (is_real_quoter(q)) + { + const lookup_info * li = lookup_with_acq_num(q); + fprintf(f, "(%s)", li ? li->name : US"???"); + } } fprintf(f, "acl%c %s %d\n%s\n", name[0], name+1, Ustrlen(value), value); } diff --git a/src/src/drtables.c b/src/src/drtables.c index 1d8e222e2..d25db887f 100644 --- a/src/src/drtables.c +++ b/src/src/drtables.c @@ -18,8 +18,9 @@ various macros in config.h that ultimately come from Local/Makefile. They are all described in src/EDITME. */ -lookup_info **lookup_list; -int lookup_list_count = 0; +//lookup_info **lookup_list; +tree_node * lookups_tree = NULL; +unsigned lookup_list_count = 0; /* Lists of information about which drivers are included in the exim binary. */ @@ -225,54 +226,51 @@ return g; -struct lookupmodulestr +static void +add_lookup_to_tree(lookup_info * li) { - void *dl; - struct lookup_module_info *info; - struct lookupmodulestr *next; -}; +tree_node * new = store_get_perm(sizeof(tree_node) + Ustrlen(li->name), + GET_UNTAINTED); +new->data.ptr = (void *)li; +Ustrcpy(new->name, li->name); +if (tree_insertnode(&lookups_tree, new)) + li->acq_num = lookup_list_count++; +else + log_write(0, LOG_MAIN|LOG_PANIC, "Duplicate lookup name '%s'", li->name); +} -static struct lookupmodulestr *lookupmodules = NULL; +/* Add all the lookup types provided by the module */ static void -addlookupmodule(void *dl, struct lookup_module_info *info) +addlookupmodule(const struct lookup_module_info * lmi) { -struct lookupmodulestr * p = - store_get(sizeof(struct lookupmodulestr), GET_UNTAINTED); - -p->dl = dl; -p->info = info; -p->next = lookupmodules; -lookupmodules = p; -lookup_list_count += info->lookupcount; +for (int j = 0; j < lmi->lookupcount; j++) + add_lookup_to_tree(lmi->lookups[j]); } -/* only valid after lookup_list and lookup_list_count are assigned */ -static void -add_lookup_to_list(lookup_info *info) -{ -/* need to add the lookup to lookup_list, sorted */ -int pos = 0; -/* strategy is to go through the list until we find -either an empty spot or a name that is higher. -this can't fail because we have enough space. */ -while (lookup_list[pos] && (Ustrcmp(lookup_list[pos]->name, info->name) <= 0)) - pos++; +static unsigned hunt_acq; -if (lookup_list[pos]) - { - /* need to insert it, so move all the other items up - (last slot is still empty, of course) */ +static void +acq_cb(uschar * name, uschar * ptr, void * ctx) +{ +lookup_info * li = (lookup_info *)ptr; +if (li->acq_num == hunt_acq) *(lookup_info **)ctx = li; +} - memmove(&lookup_list[pos+1], &lookup_list[pos], - sizeof(lookup_info *) * (lookup_list_count-pos-1)); - } -lookup_list[pos] = info; +/*XXX many of the calls here could instead use a name on the quoted-pool */ +const lookup_info * +lookup_with_acq_num(unsigned k) +{ +const lookup_info * li = NULL; +hunt_acq = k; +tree_walk(lookups_tree, acq_cb, &li); +return li; } + /* These need to be at file level for old versions of gcc (2.95.2 reported), which give parse errors on an extern in function scope. Each entry needs to also be invoked in init_lookup_list() below */ @@ -351,98 +349,96 @@ int countmodules = 0; int moduleerrors = 0; #endif static BOOL lookup_list_init_done = FALSE; -rmark reset_point; if (lookup_list_init_done) return; -reset_point = store_mark(); lookup_list_init_done = TRUE; #if defined(LOOKUP_CDB) && LOOKUP_CDB!=2 -addlookupmodule(NULL, &cdb_lookup_module_info); +addlookupmodule(&cdb_lookup_module_info); #endif #if defined(LOOKUP_DBM) && LOOKUP_DBM!=2 -addlookupmodule(NULL, &dbmdb_lookup_module_info); +addlookupmodule(&dbmdb_lookup_module_info); #endif #if defined(LOOKUP_DNSDB) && LOOKUP_DNSDB!=2 -addlookupmodule(NULL, &dnsdb_lookup_module_info); +addlookupmodule(&dnsdb_lookup_module_info); #endif #if defined(LOOKUP_DSEARCH) && LOOKUP_DSEARCH!=2 -addlookupmodule(NULL, &dsearch_lookup_module_info); +addlookupmodule(&dsearch_lookup_module_info); #endif #if defined(LOOKUP_IBASE) && LOOKUP_IBASE!=2 -addlookupmodule(NULL, &ibase_lookup_module_info); +addlookupmodule(&ibase_lookup_module_info); #endif #if defined(LOOKUP_LDAP) && LOOKUP_LDAP!=2 -addlookupmodule(NULL, &ldap_lookup_module_info); +addlookupmodule(&ldap_lookup_module_info); #endif #if defined(LOOKUP_JSON) && LOOKUP_JSON!=2 -addlookupmodule(NULL, &json_lookup_module_info); +addlookupmodule(&json_lookup_module_info); #endif #if defined(LOOKUP_LSEARCH) && LOOKUP_LSEARCH!=2 -addlookupmodule(NULL, &lsearch_lookup_module_info); +addlookupmodule(&lsearch_lookup_module_info); #endif #if defined(LOOKUP_MYSQL) && LOOKUP_MYSQL!=2 -addlookupmodule(NULL, &mysql_lookup_module_info); +addlookupmodule(&mysql_lookup_module_info); #endif #if defined(LOOKUP_NIS) && LOOKUP_NIS!=2 -addlookupmodule(NULL, &nis_lookup_module_info); +addlookupmodule(&nis_lookup_module_info); #endif #if defined(LOOKUP_NISPLUS) && LOOKUP_NISPLUS!=2 -addlookupmodule(NULL, &nisplus_lookup_module_info); +addlookupmodule(&nisplus_lookup_module_info); #endif #if defined(LOOKUP_ORACLE) && LOOKUP_ORACLE!=2 -addlookupmodule(NULL, &oracle_lookup_module_info); +addlookupmodule(&oracle_lookup_module_info); #endif #if defined(LOOKUP_PASSWD) && LOOKUP_PASSWD!=2 -addlookupmodule(NULL, &passwd_lookup_module_info); +addlookupmodule(&passwd_lookup_module_info); #endif #if defined(LOOKUP_PGSQL) && LOOKUP_PGSQL!=2 -addlookupmodule(NULL, &pgsql_lookup_module_info); +addlookupmodule(&pgsql_lookup_module_info); #endif #if defined(LOOKUP_REDIS) && LOOKUP_REDIS!=2 -addlookupmodule(NULL, &redis_lookup_module_info); +addlookupmodule(&redis_lookup_module_info); #endif #if defined(LOOKUP_LMDB) && LOOKUP_LMDB!=2 -addlookupmodule(NULL, &lmdb_lookup_module_info); +addlookupmodule(&lmdb_lookup_module_info); #endif #ifdef SUPPORT_SPF -addlookupmodule(NULL, &spf_lookup_module_info); +addlookupmodule(&spf_lookup_module_info); #endif #if defined(LOOKUP_SQLITE) && LOOKUP_SQLITE!=2 -addlookupmodule(NULL, &sqlite_lookup_module_info); +addlookupmodule(&sqlite_lookup_module_info); #endif #if defined(LOOKUP_TESTDB) && LOOKUP_TESTDB!=2 -addlookupmodule(NULL, &testdb_lookup_module_info); +addlookupmodule(&testdb_lookup_module_info); #endif #if defined(LOOKUP_WHOSON) && LOOKUP_WHOSON!=2 -addlookupmodule(NULL, &whoson_lookup_module_info); +addlookupmodule(&whoson_lookup_module_info); #endif /* This is a custom expansion, and not available as either a list-syntax lookup or a lookup expansion. However, it is implemented by a lookup module. */ -addlookupmodule(NULL, &readsock_lookup_module_info); +addlookupmodule(&readsock_lookup_module_info); #ifdef LOOKUP_MODULE_DIR if (!(dd = exim_opendir(CUS LOOKUP_MODULE_DIR))) @@ -513,7 +509,7 @@ else continue; } - addlookupmodule(dl, info); + addlookupmodule(info); DEBUG(D_lookup) debug_printf("Loaded \"%s\" (%d lookup types)\n", name, info->lookupcount); countmodules++; } @@ -527,16 +523,6 @@ DEBUG(D_lookup) debug_printf("Loaded %d lookup modules\n", countmodules); DEBUG(D_lookup) debug_printf("Total %d lookups\n", lookup_list_count); -lookup_list = store_malloc(sizeof(lookup_info *) * lookup_list_count); -memset(lookup_list, 0, sizeof(lookup_info *) * lookup_list_count); - -/* now add all lookups to the real list */ -for (struct lookupmodulestr * p = lookupmodules; p; p = p->next) - for (int j = 0; j < p->info->lookupcount; j++) - add_lookup_to_list(p->info->lookups[j]); -store_reset(reset_point); -/* just to be sure */ -lookupmodules = NULL; } #endif /*!MACRO_PREDEF*/ diff --git a/src/src/exim.c b/src/src/exim.c index 3d0929592..1a22c9af2 100644 --- a/src/src/exim.c +++ b/src/src/exim.c @@ -1183,6 +1183,17 @@ return g; } +static void +lookup_version_report_cb(uschar * name, uschar * ptr, void * ctx) +{ +const lookup_info * li = (lookup_info *)ptr; +gstring ** gp = ctx; + +if (li->version_report) + *gp = li->version_report(*gp); +} + + /* This function is called for -bV/--version and for -d to output the optional features of the current Exim binary. @@ -1368,7 +1379,7 @@ DEBUG(D_any) gnu_get_libc_version()); #endif -g = show_db_version(g); + g = show_db_version(g); #ifndef DISABLE_TLS g = tls_version_report(g); @@ -1383,12 +1394,12 @@ g = show_db_version(g); g = spf_lib_version_report(g); #endif -show_string(is_stdout, g); -g = NULL; + show_string(is_stdout, g); + g = NULL; -for (auth_info * ai = auths_available; ai; ai = (auth_info *)ai->drinfo.next) - if (ai->version_report) - g = (*ai->version_report)(g); + for (auth_info * ai = auths_available; ai; ai = (auth_info *)ai->drinfo.next) + if (ai->version_report) + g = (*ai->version_report)(g); /* PCRE_PRERELEASE is either defined and empty or a bare sequence of characters; unless it's an ancient version of PCRE in which case it @@ -1398,27 +1409,25 @@ for (auth_info * ai = auths_available; ai; ai = (auth_info *)ai->drinfo.next) #endif #define QUOTE(X) #X #define EXPAND_AND_QUOTE(X) QUOTE(X) - { - uschar buf[24]; - pcre2_config(PCRE2_CONFIG_VERSION, buf); - g = string_fmt_append(g, "Library version: PCRE2: Compile: %d.%d%s\n" - " Runtime: %s\n", - PCRE2_MAJOR, PCRE2_MINOR, - EXPAND_AND_QUOTE(PCRE2_PRERELEASE) "", - buf); - } + { + uschar buf[24]; + pcre2_config(PCRE2_CONFIG_VERSION, buf); + g = string_fmt_append(g, "Library version: PCRE2: Compile: %d.%d%s\n" + " Runtime: %s\n", + PCRE2_MAJOR, PCRE2_MINOR, + EXPAND_AND_QUOTE(PCRE2_PRERELEASE) "", + buf); + } #undef QUOTE #undef EXPAND_AND_QUOTE -show_string(is_stdout, g); -g = NULL; + show_string(is_stdout, g); + g = NULL; -init_lookup_list(); -for (int i = 0; i < lookup_list_count; i++) - if (lookup_list[i]->version_report) - g = lookup_list[i]->version_report(g); -show_string(is_stdout, g); -g = NULL; + init_lookup_list(); + tree_walk(lookups_tree, lookup_version_report_cb, &g); + show_string(is_stdout, g); + g = NULL; #ifdef WHITELIST_D_MACROS g = string_fmt_append(g, "WHITELIST_D_MACROS: \"%s\"\n", WHITELIST_D_MACROS); diff --git a/src/src/expand.c b/src/src/expand.c index e17c21788..08d72f213 100644 --- a/src/src/expand.c +++ b/src/src/expand.c @@ -2804,9 +2804,11 @@ switch(cond_type = identify_operator(&s, &opname)) case ECOND_LDAPAUTH: #ifdef LOOKUP_LDAP { - int stype = search_findtype(US"ldapauth", 8), expand_setup = -1; - void * handle = search_open(NULL, stype, 0, NULL, NULL); - if (handle) + int expand_setup = -1; + const lookup_info * li = search_findtype(US"ldapauth", 8); + void * handle; + + if (li && (handle = search_open(NULL, li, 0, NULL, NULL))) rc = search_find(handle, NULL, sub[0], -1, NULL, 0, 0, &expand_setup, NULL) ? OK : f.search_find_defer ? DEFER : FAIL; @@ -5018,9 +5020,9 @@ while (*s) case EITEM_LOOKUP: { - int stype, partial, affixlen, starflags; - int expand_setup = 0; - int nameptr = 0; + int expand_setup = 0, nameptr = 0; + int partial, affixlen, starflags; + const lookup_info * li; uschar * key, * filename; const uschar * affix, * opts; uschar * save_lookup_value = lookup_value; @@ -5073,8 +5075,8 @@ while (*s) /* Now check for the individual search type and any partial or default options. Only those types that are actually in the binary are valid. */ - if ((stype = search_findtype_partial(name, &partial, &affix, &affixlen, - &starflags, &opts)) < 0) + if (!(li = search_findtype_partial(name, &partial, &affix, &affixlen, + &starflags, &opts))) { expand_string_message = search_error_message; goto EXPAND_FAILED; @@ -5083,7 +5085,7 @@ while (*s) /* Check that a key was provided for those lookup types that need it, and was not supplied for those that use the query style. */ - if (!mac_islookup(stype, lookup_querystyle|lookup_absfilequery)) + if (!mac_islookup(li, lookup_querystyle|lookup_absfilequery)) { if (!key) { @@ -5126,7 +5128,7 @@ while (*s) file types, the query (i.e. "key") starts with a file name. */ if (!key) - key = search_args(stype, name, filename, &filename, opts); + key = search_args(li, name, filename, &filename, opts); /* If skipping, don't do the next bit - just lookup_value == NULL, as if the entry was not found. Note that there is no search_close() function. @@ -5145,7 +5147,7 @@ while (*s) lookup_value = NULL; else { - void * handle = search_open(filename, stype, 0, NULL, NULL); + void * handle = search_open(filename, li, 0, NULL, NULL); if (!handle) { expand_string_message = search_error_message; @@ -5526,12 +5528,18 @@ while (*s) if (!(flags & ESI_SKIPPING)) { - int stype = search_findtype(US"readsock", 8); + const lookup_info * li = search_findtype(US"readsock", 8); gstring * g = NULL; void * handle; int expand_setup = -1; uschar * s; + if (!li) + { + expand_string_message = search_error_message; + goto EXPAND_FAILED; + } + /* If the reqstr is empty, flag that and set a dummy */ if (!sub_arg[1][0]) @@ -5569,7 +5577,7 @@ while (*s) /* Gat a (possibly cached) handle for the connection */ - if (!(handle = search_open(sub_arg[0], stype, 0, NULL, NULL))) + if (!(handle = search_open(sub_arg[0], li, 0, NULL, NULL))) { if (*expand_string_message) goto EXPAND_FAILED; expand_string_message = search_error_message; @@ -7792,19 +7800,19 @@ NOT_ITEM: ; else { - int n; + const lookup_info * li; uschar * opt = Ustrchr(arg, '_'); if (opt) *opt++ = 0; - if ((n = search_findtype(arg, Ustrlen(arg))) < 0) + if (!(li = search_findtype(arg, Ustrlen(arg)))) { expand_string_message = search_error_message; goto EXPAND_FAILED; } - if (lookup_list[n]->quote) - sub = (lookup_list[n]->quote)(sub, opt, (unsigned)n); + if (li->quote) + sub = (li->quote)(sub, opt, li->acq_num); else if (opt) sub = NULL; diff --git a/src/src/functions.h b/src/src/functions.h index 9ae51d05b..b97895b8a 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -338,6 +338,8 @@ extern int ipv6_nmtoa(int *, uschar *); extern const uschar *local_part_quote(const uschar *); extern int log_open_as_exim(const uschar * const); extern void log_close_all(void); +extern const lookup_info * lookup_with_acq_num(unsigned); + extern macro_item * macro_create(const uschar *, const uschar *, BOOL); extern BOOL macro_read_assignment(uschar *); @@ -490,13 +492,15 @@ extern gstring * route_show_supported(gstring *); extern void route_tidyup(void); extern uschar *router_current_name(void); -extern uschar *search_args(int, uschar *, uschar *, uschar **, const uschar *); +extern uschar *search_args(const lookup_info *, uschar *, uschar *, uschar **, + const uschar *); extern uschar *search_find(void *, const uschar *, const uschar *, int, const uschar *, int, int, int *, const uschar *); -extern int search_findtype(const uschar *, int); -extern int search_findtype_partial(const uschar *, int *, const uschar **, int *, - int *, const uschar **); -extern void *search_open(const uschar *, int, int, uid_t *, gid_t *); +extern const lookup_info * search_findtype(const uschar *, int); +extern const lookup_info * search_findtype_partial(const uschar *, int *, + const uschar **, int *, int *, const uschar **); +extern void *search_open(const uschar *, const lookup_info *, int, + uid_t *, gid_t *); extern void search_tidyup(void); extern BOOL send_fd_over_socket(int, int); extern uschar *sender_helo_verified_boolstr(void); diff --git a/src/src/globals.h b/src/src/globals.h index 9731b7b58..a542a179f 100644 --- a/src/src/globals.h +++ b/src/src/globals.h @@ -729,8 +729,8 @@ extern uschar *log_selector_string; /* As supplied in the config */ extern FILE *log_stderr; /* Copy of stderr for log use, or NULL */ extern BOOL log_timezone; /* TRUE to include the timezone in log lines */ extern uschar *login_sender_address; /* The actual sender address */ -extern lookup_info **lookup_list; /* Array of pointers to available lookups */ -extern int lookup_list_count; /* Number of entries in the list */ +extern tree_node *lookups_tree; /* Tree of available lookups */ +extern unsigned lookup_list_count; /* Number of entries in the list */ extern uschar *lookup_dnssec_authenticated; /* AD status of dns lookup */ extern int lookup_open_max; /* Max lookup files to cache */ extern uschar *lookup_value; /* Value looked up from file */ diff --git a/src/src/lookupapi.h b/src/src/lookupapi.h index 62c8c0524..af7bd51f6 100644 --- a/src/src/lookupapi.h +++ b/src/src/lookupapi.h @@ -19,6 +19,7 @@ typedef struct lookup_info { uschar *name; /* e.g. "lsearch" */ int type; /* query/singlekey/abs-file */ + unsigned acq_num; /* acquisition number */ void *(*open)( /* open function */ const uschar *, /* file name for those that have one */ uschar **); /* for error message */ diff --git a/src/src/lookups/ldap.c b/src/src/lookups/ldap.c index bf1e63325..ee4099419 100644 --- a/src/src/lookups/ldap.c +++ b/src/src/lookups/ldap.c @@ -1408,7 +1408,7 @@ Arguments: s the string to be quoted opt additional option text or NULL if none only "dn" is recognized - idx lookup type index + idx quoter identification number Returns: the processed string or NULL for a bad option */ diff --git a/src/src/macros.h b/src/src/macros.h index 3ccbe3353..3bfcf51d5 100644 --- a/src/src/macros.h +++ b/src/src/macros.h @@ -102,7 +102,7 @@ don't make the file descriptors two-way. */ /* A macro to simplify testing bits in lookup types */ -#define mac_islookup(a,b) ((lookup_list[a]->type & (b)) != 0) +#define mac_islookup(li,b) ((li)->type & (b)) /* Debugging control */ diff --git a/src/src/match.c b/src/src/match.c index 4f4f2c777..0b3632f7e 100644 --- a/src/src/match.c +++ b/src/src/match.c @@ -96,7 +96,8 @@ check_string(void * arg, const uschar * pattern, const uschar ** valueptr, uschar ** error) { const check_string_block * cb = arg; -int search_type, partial, affixlen, starflags; +int partial, affixlen, starflags; +const lookup_info * li; int expand_setup = cb->expand_setup; const uschar * affix, * opts; uschar *s; @@ -268,11 +269,10 @@ if ((semicolon = Ustrchr(pattern, ';')) == NULL) the part of the string preceding the semicolon. */ *semicolon = 0; -search_type = search_findtype_partial(pattern, &partial, &affix, &affixlen, +li = search_findtype_partial(pattern, &partial, &affix, &affixlen, &starflags, &opts); *semicolon = ';'; -if (search_type < 0) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "%s", - search_error_message); +if (!li) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "%s", search_error_message); /* Partial matching is not appropriate for certain lookups (e.g. when looking up user@domain for sender rejection). There's a flag to disable it. */ @@ -281,13 +281,13 @@ if (!(cb->flags & MCS_PARTIAL)) partial = -1; /* Set the parameters for the three different kinds of lookup. */ -keyquery = search_args(search_type, s, semicolon+1, &filename, opts); +keyquery = search_args(li, s, semicolon+1, &filename, opts); /* Now do the actual lookup; throw away the data returned unless it was asked for; partial matching is all handled inside search_find(). Note that there is no search_close() because of the caching arrangements. */ -if (!(handle = search_open(filename, search_type, 0, NULL, NULL))) +if (!(handle = search_open(filename, li, 0, NULL, NULL))) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "%s", search_error_message); result = search_find(handle, filename, keyquery, partial, affix, affixlen, starflags, &expand_setup, opts); diff --git a/src/src/search.c b/src/src/search.c index 1c501455e..5078920f1 100644 --- a/src/src/search.c +++ b/src/src/search.c @@ -59,44 +59,25 @@ Arguments: name lookup type name - not necessarily zero terminated (e.g. dbm*) len length of the name -Returns: +ve => valid lookup name; value is offset in lookup_list - -ve => invalid name; message in search_error_message. +Returns: ptr to info struct for the lookup, + or NULL with message in search_error_message. */ -int +const lookup_info * search_findtype(const uschar * name, int len) { -for (int bot = 0, top = lookup_list_count; top > bot; ) - { - int mid = (top + bot)/2; - int c = Ustrncmp(name, lookup_list[mid]->name, len); - - /* If c == 0 we have matched the incoming name with the start of the search - type name. However, some search types are substrings of others (e.g. nis and - nisplus) so we need to check that the lengths are the same. The length of the - type name cannot be shorter (else c would not be 0); if it is not equal it - must be longer, and in that case, the incoming name comes before the name we - are testing. By leaving c == 0 when the lengths are different, and doing a - > 0 test below, this all falls out correctly. */ - - if (c == 0 && Ustrlen(lookup_list[mid]->name) == len) - { - if (lookup_list[mid]->find != NULL) return mid; - search_error_message = string_sprintf("lookup type \"%.*s\" is not " - "available (not in the binary - check buildtime LOOKUP configuration)", - len, name); - return -1; - } +const uschar * s = name[len] ? string_copyn(name, len) : name; +tree_node * t = tree_search(lookups_tree, s); - if (c > 0) bot = mid + 1; else top = mid; - } +if (t) return t->data.ptr; -search_error_message = string_sprintf("unknown lookup type \"%.*s\"", len, name); -return -1; +search_error_message = string_sprintf("unknown lookup type \"%s\"", s); +return NULL; } + /************************************************* * Validate a full lookup type name * *************************************************/ @@ -117,18 +98,17 @@ Arguments: starflags where to put the SEARCH_STAR and SEARCH_STARAT flags opts where to put the options -Returns: +ve => valid lookup name; value is offset in lookup_list - -ve => invalid name; message in search_error_message. +Returns: ptr to info struct for the lookup, + or NULL with message in search_error_message. */ -int +const lookup_info * search_findtype_partial(const uschar *name, int *ptypeptr, const uschar **ptypeaff, int *afflen, int *starflags, const uschar ** opts) { -int len, stype; -int pv = -1; -const uschar *ss = name; -const uschar * t; +const lookup_info * li; +int pv = -1, len; +const uschar * ss = name, * t; *starflags = 0; *ptypeaff = NULL; @@ -165,7 +145,7 @@ if (Ustrncmp(name, "partial", 7) == 0) BAD_TYPE: search_error_message = string_sprintf("format error in lookup type \"%s\"", name); - return -1; + return NULL; } } @@ -194,31 +174,31 @@ else binary are valid. For query-style types, "partial" and default types are erroneous. */ -stype = search_findtype(ss, len); -if (stype >= 0 && mac_islookup(stype, lookup_querystyle)) +li = search_findtype(ss, len); +if (li && mac_islookup(li, lookup_querystyle)) { if (pv >= 0) { search_error_message = string_sprintf("\"partial\" is not permitted " "for lookup type \"%s\"", ss); - return -1; + return NULL; } if ((*starflags & (SEARCH_STAR|SEARCH_STARAT)) != 0) { search_error_message = string_sprintf("defaults using \"*\" or \"*@\" are " "not permitted for lookup type \"%s\"", ss); - return -1; + return NULL; } } *ptypeptr = pv; -return stype; +return li; } /* Set the parameters for the three different kinds of lookup. Arguments: - search_type the search-type code + li the info block for the search type search the search-type string query argument for the search; filename or query fnamep pointer to return filename @@ -227,11 +207,11 @@ Arguments: Return: keyquery the search-type (for single-key) or query (for query-type) */ uschar * -search_args(int search_type, uschar * search, uschar * query, uschar ** fnamep, +search_args(const lookup_info * li, uschar * search, uschar * query, uschar ** fnamep, const uschar * opts) { Uskip_whitespace(&query); -if (mac_islookup(search_type, lookup_absfilequery)) +if (mac_islookup(li, lookup_absfilequery)) { /* query-style but with file (sqlite) */ int sep = ','; @@ -255,7 +235,7 @@ if (mac_islookup(search_type, lookup_absfilequery)) *fnamep = NULL; return query; /* remainder after file skipped */ } -if (!mac_islookup(search_type, lookup_querystyle)) +if (!mac_islookup(li, lookup_querystyle)) { /* single-key */ *fnamep = query; return search; /* modifiers important so use "keyquery" for them */ @@ -288,11 +268,19 @@ Returns: nothing static void tidyup_subtree(tree_node *t) { -search_cache * c = (search_cache *)(t->data.ptr); +search_cache * c = (search_cache *)t->data.ptr; if (t->left) tidyup_subtree(t->left); if (t->right) tidyup_subtree(t->right); -if (c && c->handle && lookup_list[c->search_type]->close) - lookup_list[c->search_type]->close(c->handle); +if (c && c->handle && c->li->close) + c->li->close(c->handle); +} + + +static void +tidy_cb(uschar * name, uschar * ptr, void * ctx) +{ +lookup_info * li = (lookup_info *)ptr; +if (li->tidy) (li->tidy)(); } @@ -322,8 +310,7 @@ open_filecount = 0; /* Call the general tidyup entry for any drivers that have one. */ -for (int i = 0; i < lookup_list_count; i++) if (lookup_list[i]->tidy) - (lookup_list[i]->tidy)(); +tree_walk(lookups_tree, tidy_cb, NULL); if (search_reset_point) search_reset_point = store_reset(search_reset_point); store_pool = old_pool; @@ -366,7 +353,7 @@ but is marked closed. Arguments: filename the name of the file for single-key+file style lookups, NULL for query-style lookups - search_type the type of search required + li the info block for the type of search required modemask if a real single file is used, this specifies mode bits that must not be set; otherwise it is ignored owners if a real single file is used, this specifies the possible @@ -381,13 +368,12 @@ Returns: an identifying handle for the open database; */ void * -search_open(const uschar * filename, int search_type, int modemask, +search_open(const uschar * filename, const lookup_info * li, int modemask, uid_t * owners, gid_t * owngroups) { void *handle; tree_node *t; search_cache *c; -lookup_info *lk = lookup_list[search_type]; uschar keybuffer[256]; int old_pool = store_pool; @@ -403,7 +389,7 @@ if (filename && is_tainted(filename)) store_pool = POOL_SEARCH; if (!search_reset_point) search_reset_point = store_mark(); -DEBUG(D_lookup) debug_printf_indent("search_open: %s \"%s\"\n", lk->name, +DEBUG(D_lookup) debug_printf_indent("search_open: %s \"%s\"\n", li->name, filename ? filename : US"NULL"); /* See if we already have this open for this type of search, and if so, @@ -411,7 +397,7 @@ pass back the tree block as the handle. The key for the tree node is the search type plus '0' concatenated with the file name. There may be entries in the tree with closed files if a lot of files have been opened. */ -sprintf(CS keybuffer, "%c%.254s", search_type + '0', +sprintf(CS keybuffer, "%c%.254s", li->acq_num+ '0', filename ? filename : US""); if ((t = tree_search(search_tree, keybuffer))) @@ -431,7 +417,7 @@ this, if the search type is one that uses real files, check on the number that we are holding open in the cache. If the limit is reached, close the least recently used one. */ -if (lk->type == lookup_absfile && open_filecount >= lookup_open_max) +if (li->type == lookup_absfile && open_filecount >= lookup_open_max) if (!open_bot) log_write(0, LOG_MAIN|LOG_PANIC, "too many lookups open, but can't find " "one to close"); @@ -444,7 +430,7 @@ if (lk->type == lookup_absfile && open_filecount >= lookup_open_max) ((search_cache *)(open_bot->data.ptr))->down = NULL; else open_top = NULL; - ((lookup_list[c->search_type])->close)(c->handle); + ((c->li)->close)(c->handle); c->handle = NULL; open_filecount--; } @@ -452,24 +438,24 @@ if (lk->type == lookup_absfile && open_filecount >= lookup_open_max) /* If opening is successful, call the file-checking function if there is one, and if all is still well, enter the open database into the tree. */ -if (!(handle = (lk->open)(filename, &search_error_message))) +if (!(handle = (li->open)(filename, &search_error_message))) { store_pool = old_pool; return NULL; } -if ( lk->check - && !lk->check(handle, filename, modemask, owners, owngroups, +if ( li->check + && !li->check(handle, filename, modemask, owners, owngroups, &search_error_message)) { - lk->close(handle); + li->close(handle); store_pool = old_pool; return NULL; } /* If this is a search type that uses real files, keep count. */ -if (lk->type == lookup_absfile) open_filecount++; +if (li->type == lookup_absfile) open_filecount++; /* If we found a previously opened entry in the tree, re-use it; otherwise insert a new entry. On re-use, leave any cached lookup data and the lookup @@ -486,7 +472,7 @@ if (!t) else c = t->data.ptr; c->handle = handle; -c->search_type = search_type; +c->li = li; c->up = c->down = NULL; store_pool = old_pool; @@ -525,9 +511,10 @@ internal_search_find(void * handle, const uschar * filename, { tree_node * t = (tree_node *)handle; search_cache * c = (search_cache *)(t->data.ptr); +const lookup_info * li = c->li; expiring_data * e = NULL; /* compiler quietening */ uschar * data = NULL; -int search_type = t->name[0] - '0'; +int quoter_id = li->acq_num; int old_pool = store_pool; /* Lookups that return DEFER may not always set an error message. So that @@ -538,8 +525,7 @@ f.search_find_defer = FALSE; DEBUG(D_lookup) debug_printf_indent("internal_search_find: file=\"%s\"\n " "type=%s key=\"%s\" opts=%s%s%s\n", filename, - lookup_list[search_type]->name, keystring, - opts ? "\"" : "", opts, opts ? "\"" : ""); + li->name, keystring, opts ? "\"" : "", opts, opts ? "\"" : ""); /* Insurance. If the keystring is empty, just fail. */ @@ -591,11 +577,11 @@ else XXX Should we this move into lf_sqlperform() ? The server-taint check is there. Also it already knows about looking for a "servers" spec in the query string. - Passing search_type down that far is an issue. + Passing quoter_id down that far is an issue. */ - if ( !filename && lookup_list[search_type]->quote - && is_tainted(keystring) && !is_quoted_like(keystring, search_type)) + if ( !filename && li->quote + && is_tainted(keystring) && !is_quoted_like(keystring, quoter_id)) { const uschar * ks = keystring; uschar * loc = acl_current_verb(); @@ -628,9 +614,12 @@ else DEBUG(D_lookup) { int q = quoter_for_address(ks); - debug_printf_indent("search_type %d (%s) quoting %d (%s)\n", - search_type, lookup_list[search_type]->name, - q, is_real_quoter(q) ? lookup_list[q]->name : US"none"); + const lookup_info * qli = lookup_with_acq_num(q); + + debug_printf_indent("quoter_id %d (%s) quoting %d (%s)\n", + quoter_id, li->name, + q, + is_real_quoter(q) ? qli ? qli->name : US"???" : US"none"); } #endif } @@ -639,7 +628,7 @@ else like FAIL, except that search_find_defer is set so the caller can distinguish if necessary. */ - if (lookup_list[search_type]->find(c->handle, filename, keystring, keylength, + if (li->find(c->handle, filename, keystring, keylength, &data, &search_error_message, &do_cache, opts) == DEFER) f.search_find_defer = TRUE; @@ -768,33 +757,36 @@ if (opts) /* Arrange to put this database at the top of the LRU chain if it is a type that opens real files. */ -if ( open_top != (tree_node *)handle - && lookup_list[t->name[0]-'0']->type == lookup_absfile) +if (open_top != (tree_node *)handle) { - search_cache *c = (search_cache *)(t->data.ptr); - tree_node *up = c->up; - tree_node *down = c->down; + const lookup_info * li = lookup_with_acq_num(t->name[0]-'0'); + if (li && li->type == lookup_absfile) + { + search_cache *c = (search_cache *)(t->data.ptr); + tree_node *up = c->up; + tree_node *down = c->down; - /* Cut it out of the list. A newly opened file will a NULL up pointer. - Otherwise there will be a non-NULL up pointer, since we checked above that - this block isn't already at the top of the list. */ + /* Cut it out of the list. A newly opened file will a NULL up pointer. + Otherwise there will be a non-NULL up pointer, since we checked above that + this block isn't already at the top of the list. */ - if (up) - { - ((search_cache *)(up->data.ptr))->down = down; - if (down) - ((search_cache *)(down->data.ptr))->up = up; - else - open_bot = up; - } + if (up) + { + ((search_cache *)(up->data.ptr))->down = down; + if (down) + ((search_cache *)(down->data.ptr))->up = up; + else + open_bot = up; + } - /* Now put it at the head of the list. */ + /* Now put it at the head of the list. */ - c->up = NULL; - c->down = open_top; - if (!open_top) open_bot = t; - else ((search_cache *)(open_top->data.ptr))->up = t; - open_top = t; + c->up = NULL; + c->down = open_top; + if (!open_top) open_bot = t; + else ((search_cache *)(open_top->data.ptr))->up = t; + open_top = t; + } } DEBUG(D_lookup) diff --git a/src/src/spool_in.c b/src/src/spool_in.c index 4ee1455d0..76b2f615b 100644 --- a/src/src/spool_in.c +++ b/src/src/spool_in.c @@ -518,15 +518,15 @@ for (;;) for (s = ++var; *s != ')'; ) s++; #ifndef COMPILE_UTILITY { - int idx; - if ((idx = search_findtype(var, s - var)) < 0) + const lookup_info * li; + if (!(li= search_findtype(var, s - var))) { DEBUG(D_any) debug_printf("Unrecognised quoter %.*s\n", (int)(s - var), var+1); where = NULL; goto SPOOL_FORMAT_ERROR; } - proto_mem = store_get_quoted(1, GET_TAINTED, idx); + proto_mem = store_get_quoted(1, GET_TAINTED, li->acq_num); } #endif /* COMPILE_UTILITY */ var = s + 1; diff --git a/src/src/spool_out.c b/src/src/spool_out.c index 574aa167f..e1972aa44 100644 --- a/src/src/spool_out.c +++ b/src/src/spool_out.c @@ -126,7 +126,11 @@ if (is_tainted(val)) { int q = quoter_for_address(val); putc('-', fp); - if (is_real_quoter(q)) fprintf(fp, "(%s)", lookup_list[q]->name); + if (is_real_quoter(q)) + { + const lookup_info * li = lookup_with_acq_num(q); + fprintf(fp, "(%s)", li ? li->name : US"unknown!"); + } } fprintf(fp, "%s %s\n", name, val); } diff --git a/src/src/store.c b/src/src/store.c index 181e7fd67..09f812dfd 100644 --- a/src/src/store.c +++ b/src/src/store.c @@ -651,8 +651,11 @@ BOOL is_quoted_like(const void * p, unsigned quoter) { int pq = quoter_for_address(p); -BOOL y = - is_real_quoter(pq) && lookup_list[pq]->quote == lookup_list[quoter]->quote; +const lookup_info * p_li = lookup_with_acq_num(pq); +void * p_qfn = p_li ? p_li->quote : NULL; +const lookup_info * q_li = lookup_with_acq_num(quoter); +void * q_qfn = q_li ? q_li->quote : NULL; +BOOL y = is_real_quoter(pq) && p_qfn == q_qfn; /* debug_printf("is_quoted(%p, %u): %c\n", p, quoter, y?'T':'F'); */ return y; } @@ -1254,8 +1257,10 @@ DEBUG(D_memory) for (quoted_pooldesc * qp = quoted_pools; qp; i++, qp = qp->next) { pooldesc * pp = &qp->pool; + const lookup_info* li = lookup_with_acq_num(qp->quoter); debug_printf("----Exit pool Q%d max: %3d kB in %d blocks at order %u\ttainted quoted:%s\n", - i, (pp->maxbytes+1023)/1024, pp->maxblocks, pp->maxorder, lookup_list[qp->quoter]->name); + i, (pp->maxbytes+1023)/1024, pp->maxblocks, pp->maxorder, + li ? li->name : US"???"); } } #endif @@ -1297,7 +1302,11 @@ debug_print_taint(const void * p) int q = quoter_for_address(p); if (!is_tainted(p)) return; debug_printf("(tainted"); -if (is_real_quoter(q)) debug_printf(", quoted:%s", lookup_list[q]->name); +if (is_real_quoter(q)) + { + const lookup_info * li = lookup_with_acq_num(q); + debug_printf(", quoted:%s", li ? li->name : US"???"); + } debug_printf(")\n"); } #endif diff --git a/src/src/structs.h b/src/src/structs.h index ffcd8a5df..702c7c72c 100644 --- a/src/src/structs.h +++ b/src/src/structs.h @@ -742,7 +742,7 @@ to close. */ typedef struct search_cache { void *handle; /* lookup handle, or NULL if closed */ - int search_type; /* search type */ + const lookup_info *li; /* info struct for search type */ tree_node *up; /* LRU up pointer */ tree_node *down; /* LRU down pointer */ tree_node *item_cache; /* tree of cached results */ diff --git a/src/src/tree.c b/src/src/tree.c index 8f73a46aa..5cb44b01d 100644 --- a/src/src/tree.c +++ b/src/src/tree.c @@ -191,7 +191,7 @@ node->balance = 0; /* Deal with an empty tree */ -if (p == NULL) +if (!p) { *treebase = node; return TRUE; @@ -214,9 +214,9 @@ for (;;) /* Deal with climbing down the tree, exiting from the loop when we reach a leaf. */ - q = (c > 0)? &(p->right) : &(p->left); + q = c > 0 ? &p->right : &p->left; p = *q; - if (p == NULL) break; + if (!p) break; /* Save the address of the pointer to the last node en route which has a non-zero balance factor. */ diff --git a/src/src/verify.c b/src/src/verify.c index 3f3071223..f936de50a 100644 --- a/src/src/verify.c +++ b/src/src/verify.c @@ -3060,7 +3060,7 @@ else if (iplookup) { int insize; - int search_type; + const lookup_info * li; int incoming[4]; void *handle; uschar *filename, *key, *result; @@ -3068,10 +3068,8 @@ if (iplookup) /* Find the search type */ - search_type = search_findtype(t, endname - t); - - if (search_type < 0) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "%s", - search_error_message); + if (!(li = search_findtype(t, endname - t))) + log_write(0, LOG_MAIN|LOG_PANIC_DIE, "%s", search_error_message); /* Adjust parameters for the type of lookup. For a query-style lookup, there is no file name, and the "key" is just the query. For query-style with a file @@ -3081,7 +3079,7 @@ if (iplookup) dot separators instead of colons, except when the lookup type is "iplsearch". */ - if (mac_islookup(search_type, lookup_absfilequery)) + if (mac_islookup(li, lookup_absfilequery)) { filename = semicolon + 1; key = filename; @@ -3089,14 +3087,14 @@ if (iplookup) filename = string_copyn(filename, key - filename); Uskip_whitespace(&key); } - else if (mac_islookup(search_type, lookup_querystyle)) + else if (mac_islookup(li, lookup_querystyle)) { filename = NULL; key = semicolon + 1; } else /* Single-key style */ { - int sep = (Ustrcmp(lookup_list[search_type]->name, "iplsearch") == 0)? + int sep = (Ustrcmp(li->name, "iplsearch") == 0)? ':' : '.'; insize = host_aton(cb->host_address, incoming); host_mask(insize, incoming, mlen); @@ -3108,7 +3106,7 @@ if (iplookup) /* Now do the actual lookup; note that there is no search_close() because of the caching arrangements. */ - if (!(handle = search_open(filename, search_type, 0, NULL, NULL))) + if (!(handle = search_open(filename, li, 0, NULL, NULL))) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "%s", search_error_message); result = search_find(handle, filename, key, -1, NULL, 0, 0, NULL, opts); @@ -3182,20 +3180,21 @@ on spec. */ if ((semicolon = Ustrchr(ss, ';'))) { const uschar * affix, * opts; - int partial, affixlen, starflags, id; + int partial, affixlen, starflags; + const lookup_info * li; *semicolon = 0; - id = search_findtype_partial(ss, &partial, &affix, &affixlen, &starflags, + li = search_findtype_partial(ss, &partial, &affix, &affixlen, &starflags, &opts); *semicolon=';'; - if (id < 0) /* Unknown lookup type */ + if (!li) /* Unknown lookup type */ { log_write(0, LOG_MAIN|LOG_PANIC, "%s in host list item \"%s\"", search_error_message, ss); return DEFER; } - isquery = mac_islookup(id, lookup_querystyle|lookup_absfilequery); + isquery = mac_islookup(li, lookup_querystyle|lookup_absfilequery); } if (isquery) diff --git a/test/runtest b/test/runtest index 0c9f2808b..ab8371942 100755 --- a/test/runtest +++ b/test/runtest @@ -1234,7 +1234,7 @@ RESET_AFTER_EXTRA_LINE_READ: # Lookups have a char which depends on the number of lookup types compiled in, # in stderr output. Replace with a "0". Recognising this while avoiding # other output is fragile; perhaps the debug output should be revised instead. - s%^\s+(:?closing )?\K[0-?]TESTSUITE/aux-fixed/%0TESTSUITE/aux-fixed/%g; + s%^\s+(?:closing )?\K[0-Z]TESTSUITE/aux-fixed/%0TESTSUITE/aux-fixed/%g; # drop gnutls version strings next if /GnuTLS compile-time version: \d+[\.\d]+$/; @@ -1471,7 +1471,7 @@ RESET_AFTER_EXTRA_LINE_READ: } # Different builds will have different lookup types included - s/search_type \K\d+ \((\w+)\) quoting -1 \(none\)$/NN ($1) quoting -1 (none)/; + s/quoter_id \K\d+ \((\w+)\) quoting -1 \(none\)$/NN ($1) quoting -1 (none)/; # and different numbers of lookup types result in different type-code letters, # so convert them all to "0" s%(?