From 729e56dab8ffd073a881fdd25db92add2a8c129b Mon Sep 17 00:00:00 2001 From: Wietse Z Venema Date: Sat, 28 Jun 2025 00:00:00 -0500 Subject: [PATCH] postfix-3.11-20250628-nonprod --- postfix/HISTORY | 23 ++++++++++ postfix/makedefs | 2 +- postfix/proto/stop.double-history | 1 + postfix/proto/stop.spell-cc | 1 + postfix/src/global/data_redirect.c | 2 +- postfix/src/global/mail_version.h | 2 +- postfix/src/global/maps.c | 17 +++---- postfix/src/global/mynetworks.c | 5 +- postfix/src/global/own_inet_addr.c | 4 +- postfix/src/global/server_acl.c | 22 ++++++--- postfix/src/postconf/postconf_dbms.c | 16 +++++-- postfix/src/proxymap/proxymap.c | 8 ++++ postfix/src/util/Makefile.in | 6 +-- postfix/src/util/dict.c | 68 ++++++++++++++++++++++++++-- postfix/src/util/dict.h | 11 +++++ postfix/src/util/dict_alloc.c | 13 ++++++ postfix/src/util/dict_debug.c | 23 +++------- postfix/src/util/dict_debug_test.sh | 0 postfix/src/util/dict_inline.c | 2 +- postfix/src/util/dict_open.c | 58 ++++++++++++++++++++---- postfix/src/util/dict_pipe.c | 24 ++++------ postfix/src/util/dict_seq.ref | 18 +++----- postfix/src/util/dict_test.c | 1 - postfix/src/util/dict_thash.c | 2 +- postfix/src/util/dict_union.c | 24 ++++------ postfix/src/util/dict_utf8_test.ref | 2 +- postfix/src/util/match_list.c | 10 ++-- 27 files changed, 254 insertions(+), 111 deletions(-) mode change 100644 => 100755 postfix/src/util/dict_debug_test.sh diff --git a/postfix/HISTORY b/postfix/HISTORY index f103cac51..bdc4353a4 100644 --- a/postfix/HISTORY +++ b/postfix/HISTORY @@ -29309,3 +29309,26 @@ Apologies for any names omitted. util/dict_regexp.c, util/dict_sdbm.c, util/dict_sockmap.c, util/dict_static.c, util/dict_surrogate.c, util/dict_tcp.c, util/dict_thash.c, util/dict_union.c, util/dict_unix.c. + +20250626 + + Cleanup: removed explicit dictionary life-cycle management + complexity from dict_xxx_open() and maps_create(), and + centralized it under the generic dict_open() API. Files: + global/maps.c, util/dict.c, util/dict.h, util/dict_alloc.c, + util/dict_debug.c, util/dict_inline.c, util/dict_open.c, + util/dict_pipe.c, util/dict_test.c, util/dict_thash.c, + util/dict_union.c. + +20250627 + + Temporary workaround: allow the proxymap server to continue + registering a dictionary under a legacy name, in addition + to the preferred name that it is registered under by + dict_open(). Files: util/dict.[hc], proxymap/proxymap.c. + + Cleanup: some unused test binaries failed to build. Files: + global/own_inet_addr.c, global/data_redirect.c, + global/mynetworks.c. + + Baseline is postfix-3.11-20250624. diff --git a/postfix/makedefs b/postfix/makedefs index 9bd730281..5d2a25571 100644 --- a/postfix/makedefs +++ b/postfix/makedefs @@ -979,7 +979,7 @@ CCARGS="$CCARGS -DSNAPSHOT" # Non-production: needs thorough testing, or major changes are still # needed before the code stabilizes. -#CCARGS="$CCARGS -DNONPROD" +CCARGS="$CCARGS -DNONPROD" # Workaround: prepend Postfix include files before other include files. CCARGS="-I. -I../../include $CCARGS" diff --git a/postfix/proto/stop.double-history b/postfix/proto/stop.double-history index 2fc207315..79d92176b 100644 --- a/postfix/proto/stop.double-history +++ b/postfix/proto/stop.double-history @@ -177,3 +177,4 @@ proto proto COMPATIBILITY_README html smtp smtp c tlsproxy tlsproxy c proto postconf proto rhansen rhansen org Files proto DATABASE_README html postconf Makefile in postconf postconf c + dict_open Files util dict hc proxymap proxymap c diff --git a/postfix/proto/stop.spell-cc b/postfix/proto/stop.spell-cc index 506c2d2c8..10ffc77e9 100644 --- a/postfix/proto/stop.spell-cc +++ b/postfix/proto/stop.spell-cc @@ -1864,3 +1864,4 @@ DGST DIGEST OSSL ossl +deduplicates diff --git a/postfix/src/global/data_redirect.c b/postfix/src/global/data_redirect.c index 1a8fe0f9c..b303e2b48 100644 --- a/postfix/src/global/data_redirect.c +++ b/postfix/src/global/data_redirect.c @@ -227,7 +227,7 @@ int main(int argc, char **argv) vstream_fflush(VSTREAM_OUT); continue; } - target = mystrtokq(&bufp, " \t"); + target = mystrtokq(&bufp, " \t", CHARS_BRACE); junk = mystrtok(&bufp, " \t"); if (strcmp(cmd, "file") == 0 && target && !junk) { data_redirect_file(result, target); diff --git a/postfix/src/global/mail_version.h b/postfix/src/global/mail_version.h index d8c67baf7..2396ba098 100644 --- a/postfix/src/global/mail_version.h +++ b/postfix/src/global/mail_version.h @@ -20,7 +20,7 @@ * Patches change both the patchlevel and the release date. Snapshots have no * patchlevel; they change the release date only. */ -#define MAIL_RELEASE_DATE "20250624" +#define MAIL_RELEASE_DATE "20250628" #define MAIL_VERSION_NUMBER "3.11" #ifdef SNAPSHOT diff --git a/postfix/src/global/maps.c b/postfix/src/global/maps.c index 592ec91d9..7fb33ed99 100644 --- a/postfix/src/global/maps.c +++ b/postfix/src/global/maps.c @@ -132,7 +132,6 @@ MAPS *maps_create(const char *title, const char *map_names, int dict_flags) static char parens[] = CHARS_BRACE; MAPS *maps; char *map_type_name; - VSTRING *map_type_name_flags; DICT *dict; /* @@ -149,23 +148,17 @@ MAPS *maps_create(const char *title, const char *map_names, int dict_flags) */ if (*map_names) { bufp = temp = mystrdup(map_names); - map_type_name_flags = vstring_alloc(10); #define OPEN_FLAGS O_RDONLY while ((map_type_name = mystrtokq(&bufp, sep, parens)) != 0) { - dict_make_registered_name(map_type_name_flags, map_type_name, - OPEN_FLAGS, dict_flags); - if ((dict = dict_handle(vstring_str(map_type_name_flags))) == 0) - dict = dict_open(map_type_name, OPEN_FLAGS, dict_flags); + dict = dict_open(map_type_name, OPEN_FLAGS, dict_flags); if ((dict->flags & dict_flags) != dict_flags) msg_panic("%s: map %s has flags 0%o, want flags 0%o", myname, map_type_name, dict->flags, dict_flags); - dict_register(vstring_str(map_type_name_flags), dict); - argv_add(maps->argv, vstring_str(map_type_name_flags), ARGV_END); + argv_add(maps->argv, dict->reg_name, ARGV_END); } myfree(temp); - vstring_free(map_type_name_flags); } return (maps); } @@ -299,12 +292,16 @@ const char *maps_file_find(MAPS *maps, const char *name, int flags) MAPS *maps_free(MAPS *maps) { + const char *myname = "maps_free"; + DICT *dict; char **map_name; for (map_name = maps->argv->argv; *map_name; map_name++) { if (msg_verbose) msg_info("maps_free: %s", *map_name); - dict_unregister(*map_name); + if ((dict = dict_handle(*map_name)) == 0) + msg_panic("%s: dictionary not found: %s", myname, *map_name); + dict_close(dict); } myfree(maps->title); argv_free(maps->argv); diff --git a/postfix/src/global/mynetworks.c b/postfix/src/global/mynetworks.c index 007c046cd..75cc61701 100644 --- a/postfix/src/global/mynetworks.c +++ b/postfix/src/global/mynetworks.c @@ -315,12 +315,9 @@ const char *mynetworks_host(void) #ifdef TEST #include -char *var_inet_interfaces; -char *var_mynetworks_style; - int main(int argc, char **argv) { - INET_PROTO_INFO *proto_info; + const INET_PROTO_INFO *proto_info; if (argc != 4) msg_fatal("usage: %s protocols mask_style interface_list (e.g. \"all subnet all\")", diff --git a/postfix/src/global/own_inet_addr.c b/postfix/src/global/own_inet_addr.c index ee5959f34..7c1c37b1b 100644 --- a/postfix/src/global/own_inet_addr.c +++ b/postfix/src/global/own_inet_addr.c @@ -294,11 +294,9 @@ static void inet_addr_list_print(INET_ADDR_LIST *list) } } -char *var_inet_interfaces; - int main(int argc, char **argv) { - INET_PROTO_INFO *proto_info; + const INET_PROTO_INFO *proto_info; INET_ADDR_LIST *list; if (argc != 3) diff --git a/postfix/src/global/server_acl.c b/postfix/src/global/server_acl.c index e8eb25bb2..9227377e7 100644 --- a/postfix/src/global/server_acl.c +++ b/postfix/src/global/server_acl.c @@ -122,10 +122,14 @@ SERVER_ACL *server_acl_parse(const char *extern_acl, const char *origin) char *saved_acl = mystrdup(extern_acl); SERVER_ACL *intern_acl = argv_alloc(1); char *bp = saved_acl; + VSTRING *reg_name = 0; char *acl; #define STREQ(x,y) (strcasecmp((x), (y)) == 0) #define STRNE(x,y) (strcasecmp((x), (y)) != 0) +#define OPEN_FLAGS O_RDONLY +#define DICT_FLAGS (DICT_FLAG_LOCK | DICT_FLAG_FOLD_FIX \ + | DICT_FLAG_UTF8_REQUEST) /* * Nested tables are not allowed. Tables are opened before entering the @@ -141,10 +145,13 @@ SERVER_ACL *server_acl_parse(const char *extern_acl, const char *origin) argv_add(intern_acl, SERVER_ACL_NAME_DUNNO, (char *) 0); break; } else { - if (dict_handle(acl) == 0) - dict_register(acl, dict_open(acl, O_RDONLY, DICT_FLAG_LOCK - | DICT_FLAG_FOLD_FIX - | DICT_FLAG_UTF8_REQUEST)); + if (reg_name == 0) + reg_name = vstring_alloc(100); + dict_make_registered_name(reg_name, acl, OPEN_FLAGS, + DICT_FLAGS); + if (dict_handle(STR(reg_name)) == 0) + dict_open(acl, OPEN_FLAGS, DICT_FLAGS); + acl = STR(reg_name); } } argv_add(intern_acl, acl, (char *) 0); @@ -154,6 +161,8 @@ SERVER_ACL *server_acl_parse(const char *extern_acl, const char *origin) /* * Cleanup. */ + if (reg_name) + vstring_free(reg_name); myfree(saved_acl); return (intern_acl); } @@ -212,8 +221,9 @@ int server_acl_eval(const char *client_addr, SERVER_ACL * intern_acl, if (ret != SERVER_ACL_ACT_DUNNO) return (ret); } else if (dict->error != 0) { - msg_warn("%s: %s: table lookup error -- ignoring the remainder " - "of this access list", origin, acl); + msg_warn("%s: %s:%s: table lookup error -- ignoring the " + "remainder of this access list", origin, dict->type, + dict->name); return (SERVER_ACL_ACT_ERROR); } } else if (STREQ(acl, SERVER_ACL_NAME_DUNNO)) { diff --git a/postfix/src/postconf/postconf_dbms.c b/postfix/src/postconf/postconf_dbms.c index d6fa2a2ee..166415182 100644 --- a/postfix/src/postconf/postconf_dbms.c +++ b/postfix/src/postconf/postconf_dbms.c @@ -187,13 +187,18 @@ static void pcf_check_dbms_client(int mode, const PCF_DBMS_INFO *dp, const char *value; char *dict_spec; int dir; + VSTRING *reg_name; /* * We read each database client configuration file into its own * dictionary, and nag only the first time that a file is visited. */ dict_spec = concatenate(dp->db_type, ":", cf_file, (char *) 0); - if ((dict = dict_handle(dict_spec)) == 0) { + reg_name = vstring_alloc(100); + /* This should match the dict_open3() call below. */ + dict_make_registered_name4(reg_name, DICT_TYPE_HT, dict_spec, 0, 0); + + if ((dict = dict_handle(STR(reg_name))) == 0) { struct stat st; /* @@ -202,13 +207,13 @@ static void pcf_check_dbms_client(int mode, const PCF_DBMS_INFO *dp, * files may contain passwords and should not be world-readable. * Note: dict_load_fp() nags about duplicate parameter settings. */ - dict = dict_ht_open(dict_spec, O_CREAT | O_RDWR, 0); - dict_register(dict_spec, dict); + dict = dict_open3(DICT_TYPE_HT, dict_spec, 0, 0); if ((fp = vstream_fopen(cf_file, O_RDONLY, 0)) == 0) { if (errno != EACCES) msg_warn("open \"%s\" configuration \"%s\": %m", dp->db_type, cf_file); myfree(dict_spec); + vstring_free(reg_name); return; } if (fstat(vstream_fileno(fp), &st) == 0 && !S_ISREG(st.st_mode)) { @@ -216,13 +221,15 @@ static void pcf_check_dbms_client(int mode, const PCF_DBMS_INFO *dp, dp->db_type, cf_file); myfree(dict_spec); (void) vstream_fclose(fp); + vstring_free(reg_name); return; } - dict_load_fp(dict_spec, fp); + dict_load_fp(STR(reg_name), fp); if (vstream_fclose(fp)) { msg_warn("read \"%s\" configuration \"%s\": %m", dp->db_type, cf_file); myfree(dict_spec); + vstring_free(reg_name); return; } @@ -242,6 +249,7 @@ static void pcf_check_dbms_client(int mode, const PCF_DBMS_INFO *dp, } } myfree(dict_spec); + vstring_free(reg_name); } /* pcf_register_dbms_helper - parse one possible database type:name */ diff --git a/postfix/src/proxymap/proxymap.c b/postfix/src/proxymap/proxymap.c index c0af411f6..3a00c33c1 100644 --- a/postfix/src/proxymap/proxymap.c +++ b/postfix/src/proxymap/proxymap.c @@ -843,6 +843,14 @@ int main(int argc, char **argv) */ MAIL_VERSION_STAMP_ALLOCATE; + /* + * Workaround for programs that make explicit dict_register() calls with + * a table that is already registered under a different name. This is + * safe only in programs that do not unregister or close a table that is + * registered with multiple names. + */ + dict_allow_multiple_dict_register_names = 1; + /* * XXX When invoked with the master.cf service name "proxywrite", the * proxymap daemon will allow update requests. To update a table that is diff --git a/postfix/src/util/Makefile.in b/postfix/src/util/Makefile.in index ed1df23ed..b02164811 100644 --- a/postfix/src/util/Makefile.in +++ b/postfix/src/util/Makefile.in @@ -659,13 +659,13 @@ tests: all valid_hostname_test mac_expand_test dict_test unescape_test \ binhash_test argv_test inet_prefix_top_test printable_test \ valid_utf8_string_test readlline_test quote_for_json_test \ normalize_ws_test valid_uri_scheme_test clean_ascii_cntrl_space_test \ - test_normalize_v4mapped_addr test_ossl_digest test_dict_pipe + test_normalize_v4mapped_addr test_ossl_digest test_dict_pipe \ test_dict_union dict_tests: all dict_test \ dict_pcre_tests dict_cidr_test dict_thash_test dict_static_test \ dict_inline_test dict_utf8_test dict_regexp_test \ - dict_regexp_file_test dict_cidr_file_test \ + dict_regexp_file_test dict_cidr_file_test dict_seq_test \ dict_static_file_test dict_random_test dict_random_file_test \ dict_inline_file_test dict_stream_test dict_inline_regexp_test \ dict_inline_cidr_test dict_debug_test @@ -811,7 +811,7 @@ split_qnameval_test: split_qnameval update dict_seq_test: dict_open testdb dict_seq.in dict_seq.ref rm -f testdb.db testdb.dir testdb.pag - $(SHLIB_ENV) ${VALGRIND} ./dict_open hash:testdb create sync < dict_seq.in 2>&1 | sed 's/uid=[0-9][0-9][0-9]*/uid=USER/' > dict_seq.tmp + ${HTABLE_FIX} $(SHLIB_ENV) ${VALGRIND} ./dict_open internal:testdb create sync_update < dict_seq.in 2>&1 | sed 's/uid=[0-9][0-9][0-9]*/uid=USER/' > dict_seq.tmp diff dict_seq.ref dict_seq.tmp rm -f testdb.db testdb.dir testdb.pag dict_seq.tmp diff --git a/postfix/src/util/dict.c b/postfix/src/util/dict.c index 5fcf84294..11d6ba6e1 100644 --- a/postfix/src/util/dict.c +++ b/postfix/src/util/dict.c @@ -75,6 +75,15 @@ /* const char *type_name, /* int open_flags, /* int dict_flags) +/* +/* char *dict_make_registered_name4( +/* VSTRING *out, +/* const char *type, +/* const char *name, +/* int open_flags, +/* int dict_flags) +/* +/* int dict_allow_multiple_dict_register_names; /* DESCRIPTION /* This module maintains a collection of name-value dictionaries. /* Each dictionary has its own name and has its own methods to read @@ -173,11 +182,19 @@ /* dict_flags_mask() returns the bitmask for the specified /* comma/space-separated dictionary flag names. /* -/* dict_make_registered_name() formats a dictionary type:name and -/* (initial) flag values for use in dict_register() calls. +/* dict_make_registered_name*() format a dictionary type, name, +/* and (initial) flag values for use in dict_register() calls. /* This encourages consistent sharing of dictionary instances that /* have the exact same type:name and (initial) flags. The result /* value is the string value of the \fIout\fR VSTRING buffer. +/* +/* dict_allow_multiple_dict_register_names enables a temporary +/* workaround for programs that make explicit dict_register() +/* calls with a table that is already registered under a different +/* name. Setting this to non-zero allows a dictionary to be +/* registered under multiple names. This workaround is safe only +/* in programs that do not unregister or close a table that is +/* registered with multiple names. /* TRUST AND PROVENANCE /* .ad /* .fi @@ -322,8 +339,24 @@ typedef struct { dict = node->dict; \ } while (0) + /* + * Workaround for programs that make explicit dict_register() calls with + * tables that are already registered under a different name. This is safe + * only in programs that do not unregister or close a table that is + * registered with multiple names. + */ +int dict_allow_multiple_dict_register_names = 0; + #define STR(x) vstring_str(x) +/* dict_register_close - trigger dictionary cleanup */ + +static void dict_register_close(DICT *dict) +{ + /* This will eventually call dict->saved_lose(). */ + dict_unregister(dict->reg_name); +} + /* dict_register - make association with dictionary */ void dict_register(const char *dict_name, DICT *dict_info) @@ -331,6 +364,15 @@ void dict_register(const char *dict_name, DICT *dict_info) const char *myname = "dict_register"; DICT_NODE *node; + /* + * Enforce referential integrity. + */ + if (dict_allow_multiple_dict_register_names == 0 + && dict_info->reg_name && strcmp(dict_name, dict_info->reg_name) != 0) + msg_panic("%s: '%s:%s' is already registered under '%s' and cannot " + "also be registered under '%s'", myname, dict_info->type, + dict_info->name, dict_info->reg_name, dict_name); + if (dict_table == 0) dict_table = htable_create(0); if ((node = dict_node(dict_name)) == 0) { @@ -338,6 +380,9 @@ void dict_register(const char *dict_name, DICT *dict_info) node->dict = dict_info; node->refcount = 0; htable_enter(dict_table, dict_name, (void *) node); + dict_info->reg_name = mystrdup(dict_name); + dict_info->saved_close = dict_info->close; + dict_info->close = dict_register_close; } else if (dict_info != node->dict) msg_fatal("%s: dictionary name exists: %s", myname, dict_name); node->refcount++; @@ -361,7 +406,9 @@ static void dict_node_free(void *ptr) DICT_NODE *node = (DICT_NODE *) ptr; DICT *dict = node->dict; - if (dict->close) + if (dict->saved_close) /* managed by dict_open() */ + dict->saved_close(dict); + else dict->close(dict); myfree((void *) node); } @@ -644,9 +691,9 @@ static const NAME_MASK dict_mask[] = { "fixed", DICT_FLAG_FIXED, /* fixed key map */ "pattern", DICT_FLAG_PATTERN, /* keys are patterns */ "lock", DICT_FLAG_LOCK, /* lock before access */ - "replace", DICT_FLAG_DUP_REPLACE, /* if file, replace dups */ + "dup_replace", DICT_FLAG_DUP_REPLACE, /* if file, replace dups */ "sync_update", DICT_FLAG_SYNC_UPDATE, /* if file, sync updates */ - /*"debug", DICT_FLAG_DEBUG, /* log access */ + /* "debug", DICT_FLAG_DEBUG, /* log access */ "no_regsub", DICT_FLAG_NO_REGSUB, /* disallow regexp substitution */ "no_proxy", DICT_FLAG_NO_PROXY, /* disallow proxy mapping */ "no_unauth", DICT_FLAG_NO_UNAUTH, /* disallow unauthenticated data */ @@ -690,3 +737,14 @@ char *dict_make_registered_name(VSTRING *out, const char *type_name, type_name, open_flags, dict_flags_str(dict_flags)))); } + +/* dict_make_registered_name4 - format registry name for consistent sharing */ + +char *dict_make_registered_name4(VSTRING *out, const char *type, + const char *name, + int open_flags, int dict_flags) +{ + return (STR(vstring_sprintf(out, "%s:%s(%o,%s)", + type, name, open_flags, + dict_flags_str(dict_flags)))); +} diff --git a/postfix/src/util/dict.h b/postfix/src/util/dict.h index 780f5c06d..a6eb22de7 100644 --- a/postfix/src/util/dict.h +++ b/postfix/src/util/dict.h @@ -97,6 +97,8 @@ typedef struct DICT { struct DICT_UTF8_BACKUP *utf8_backup; /* see below */ struct VSTRING *file_buf; /* dict_file_to_buf() */ struct VSTRING *file_b64; /* dict_file_to_b64() */ + char *reg_name; /* owned by dict_register() */ + void (*saved_close) (struct DICT *); /* owned by dict_register() */ } DICT; extern DICT *dict_alloc(const char *, const char *, ssize_t); @@ -282,6 +284,15 @@ extern DICT *PRINTFLIKE(5, 6) dict_surrogate(const char *, const char *, int, in * type, name, open_flags, and (initial) dict_flags. */ extern char *dict_make_registered_name(VSTRING *, const char *, int, int); +extern char *dict_make_registered_name4(VSTRING *, const char *, const char *, int, int); + + /* + * Workaround for programs that make explicit dict_register() calls with a + * table that is already registered under a different name. This is safe + * only in programs that do not unregister or close a table that is + * registered with multiple names. + */ +extern int dict_allow_multiple_dict_register_names; /* * This name is reserved for matchlist error handling. diff --git a/postfix/src/util/dict_alloc.c b/postfix/src/util/dict_alloc.c index 3285a38e3..d780ecf97 100644 --- a/postfix/src/util/dict_alloc.c +++ b/postfix/src/util/dict_alloc.c @@ -141,6 +141,10 @@ DICT *dict_alloc(const char *dict_type, const char *dict_name, ssize_t size) { DICT *dict = (DICT *) mymalloc(size); + if (msg_verbose > 2) + msg_info("dict_alloc(\"%s\", \"%s\", %ld)", + dict_type, dict_name, (long) size); + dict->type = mystrdup(dict_type); dict->name = mystrdup(dict_name); dict->flags = DICT_FLAG_FIXED; @@ -162,6 +166,8 @@ DICT *dict_alloc(const char *dict_type, const char *dict_name, ssize_t size) dict->utf8_backup = 0; dict->file_buf = 0; dict->file_b64 = 0; + dict->reg_name = 0; + dict->saved_close = 0; return dict; } @@ -169,6 +175,11 @@ DICT *dict_alloc(const char *dict_type, const char *dict_name, ssize_t size) void dict_free(DICT *dict) { + if (msg_verbose > 2) + msg_info("dict_free type=\"%s\" name=\"%s\" reg_name=\"%s\")", + dict->type, dict->name, dict->reg_name ? + dict->reg_name : "(null)"); + myfree(dict->type); myfree(dict->name); if (dict->jbuf) @@ -179,6 +190,8 @@ void dict_free(DICT *dict) vstring_free(dict->file_buf); if (dict->file_b64) vstring_free(dict->file_b64); + if (dict->reg_name) + myfree(dict->reg_name); myfree((void *) dict); } diff --git a/postfix/src/util/dict_debug.c b/postfix/src/util/dict_debug.c index 8e2af64fd..8fde809e0 100644 --- a/postfix/src/util/dict_debug.c +++ b/postfix/src/util/dict_debug.c @@ -128,8 +128,9 @@ static int dict_debug_sequence(DICT *dict, int function, static void dict_debug_close(DICT *dict) { - /* TODO(wietse) use the annotated name from dict_make_registered_name(). */ - dict_unregister(dict->name); + DICT_DEBUG *dict_debug = (DICT_DEBUG *) dict; + + dict_close(dict_debug->real_dict); dict_free(dict); } @@ -143,21 +144,11 @@ DICT *dict_debug_open(const char *name, int open_flags, int dict_flags) msg_info("%s: %s", myname, name); /* - * Reuse a previously registered table if present. This prevents a config - * containing both debug:foo:bar and foo:bar from creating two DICT - * objects for foo:bar. - * - * TODO(wietse) use the annotated name from dict_make_registered_name(). - */ - DICT *real_dict = dict_handle(name); - - if (real_dict == 0) - real_dict = dict_open(name, open_flags, dict_flags); - dict_register(name, real_dict); - - /* - * Encapsulate the real dictionary. + * dict_open() will reuse a previously registered table if present. This + * prevents a config containing both debug:foo:bar and foo:bar from + * creating two DICT objects for foo:bar. */ + DICT *real_dict = dict_open(name, open_flags, dict_flags); DICT_DEBUG *dict_debug = (DICT_DEBUG *) dict_alloc(DICT_TYPE_DEBUG, name, sizeof(*dict_debug)); diff --git a/postfix/src/util/dict_debug_test.sh b/postfix/src/util/dict_debug_test.sh old mode 100644 new mode 100755 diff --git a/postfix/src/util/dict_inline.c b/postfix/src/util/dict_inline.c index ca3c94de2..d0bee90d7 100644 --- a/postfix/src/util/dict_inline.c +++ b/postfix/src/util/dict_inline.c @@ -110,7 +110,7 @@ DICT *dict_inline_open(const char *name, int open_flags, int dict_flags) /* * Reuse the "internal" dictionary type. */ - dict = dict_open3(DICT_TYPE_HT, name, open_flags, dict_flags); + dict = dict_ht_open(name, open_flags, dict_flags); dict_type_override(dict, DICT_TYPE_INLINE); while ((nameval = mystrtokq(&cp, CHARS_COMMA_SP, CHARS_BRACE)) != 0) { if (nameval[0] == CHARS_BRACE[0]) diff --git a/postfix/src/util/dict_open.c b/postfix/src/util/dict_open.c index bc42c06fc..93fed1fcc 100644 --- a/postfix/src/util/dict_open.c +++ b/postfix/src/util/dict_open.c @@ -82,7 +82,17 @@ /* const char *type; /* DESCRIPTION /* This module implements a low-level interface to multiple -/* physical dictionary types. +/* dictionary types. +/* +/* In addition to providing a mapping from type names to +/* implementations, this module deduplicates requests to open a +/* dictionary with the same fingerprint (type, name, and initial +/* flags), and manages the dictionary life cycle using reference +/* counts maintained with dict_(un)register(). +/* +/* The fingerprint, generated with dict_make_registered_name() +/* and available as DICT.reg_name, may be used in dict_handle() +/* calls. /* /* dict_open() takes a type:name pair that specifies a dictionary type /* and dictionary name, opens the dictionary, and returns a dictionary @@ -164,8 +174,6 @@ /* Enable preliminary code for bulk-mode database updates. /* The caller must create an exception handler with dict_jmp_alloc() /* and must trap exceptions from the database client with dict_setjmp(). -/* .IP DICT_FLAG_DEBUG -/* Enable additional logging. /* .IP DICT_FLAG_UTF8_REQUEST /* With util_utf8_enable != 0, require that lookup/update/delete /* keys and values are valid UTF-8. Skip a lookup/update/delete @@ -281,7 +289,11 @@ /* /* dict_type_override() changes the symbolic dictionary type. /* This is used by dictionaries whose internals are based on -/* some other dictionary type. +/* some other dictionary type. dict_type_override() requires that +/* the dictionary is not already registered with dict_register(), +/* i.e., it must be the result from dict_xxx_open(), not from +/* dict_open(). If needed in the future, this limitation may +/* be lifted. /* DIAGNOSTICS /* Fatal error: open error, unsupported dictionary type, attempt to /* update non-writable dictionary. @@ -481,19 +493,36 @@ DICT *dict_open3(const char *dict_type, const char *dict_name, { const char *myname = "dict_open"; const DICT_OPEN_INFO *dp; + VSTRING *reg_name = vstring_alloc(100); DICT *dict; +#define DICT_OPEN3_RETURN(d) do { \ + DICT *_d = (d); \ + dict_register(vstring_str(reg_name), _d); \ + vstring_free(reg_name); \ + return (_d); \ + } while (0) + + /* + * If the dictionary is already open, simply increase the reference count + * to update an existing life cycle. + */ + dict_make_registered_name4(reg_name, dict_type, dict_name, + open_flags, dict_flags); + if ((dict = dict_handle(vstring_str(reg_name))) != 0) + DICT_OPEN3_RETURN(dict); + if (*dict_type == 0 || *dict_name == 0) msg_fatal("open dictionary: expecting \"type:name\" form instead of \"%s:%s\"", dict_type, dict_name); if (NEED_DICT_OPEN_INIT()) dict_open_init(); if ((dp = dict_open_lookup(dict_type)) == 0) - return (dict_surrogate(dict_type, dict_name, open_flags, dict_flags, - "unsupported dictionary type: %s", dict_type)); + DICT_OPEN3_RETURN(dict_surrogate(dict_type, dict_name, open_flags, + dict_flags, "unsupported dictionary type: %s", dict_type)); if ((dict = dp->dict_fn(dict_name, open_flags, dict_flags)) == 0) - return (dict_surrogate(dict_type, dict_name, open_flags, dict_flags, - "cannot open %s:%s: %m", dict_type, dict_name)); + DICT_OPEN3_RETURN(dict_surrogate(dict_type, dict_name, open_flags, + dict_flags, "cannot open %s:%s: %m", dict_type, dict_name)); if (msg_verbose) msg_info("%s: %s:%s", myname, dict_type, dict_name); /* XXX The choice between wait-for-lock or no-wait is hard-coded. */ @@ -515,7 +544,8 @@ DICT *dict_open3(const char *dict_type, const char *dict_name, if ((dict->flags & DICT_FLAG_UTF8_ACTIVE) == 0 && DICT_NEED_UTF8_ACTIVATION(util_utf8_enable, dict_flags)) dict = dict_utf8_activate(dict); - return (dict); + /* Register the result. */ + DICT_OPEN3_RETURN(dict); } /* dict_open_register - register dictionary type */ @@ -601,6 +631,16 @@ DICT_MAPNAMES_EXTEND_FN dict_mapnames_extend(DICT_MAPNAMES_EXTEND_FN new_cb) void dict_type_override(DICT *dict, const char *type) { + + /* + * To lift this limitation, compute a new reg_name, and implement a move + * (copy+delete) operation from the old reg_name to the new one. Also + * handle the case that the new destination name is already in use. This + * should be encapsulated in code that is adjacent to dict_register(). + */ + if (dict->reg_name) + msg_panic("%s: %s:%s is already registered", + __func__, dict->type, dict->name); myfree(dict->type); dict->type = mystrdup(type); } diff --git a/postfix/src/util/dict_pipe.c b/postfix/src/util/dict_pipe.c index 7a4b92437..02f7c87df 100644 --- a/postfix/src/util/dict_pipe.c +++ b/postfix/src/util/dict_pipe.c @@ -90,12 +90,17 @@ static const char *dict_pipe_lookup(DICT *dict, const char *query) static void dict_pipe_close(DICT *dict) { + static const char myname[] = "dict_pipe_close"; DICT_PIPE *dict_pipe = (DICT_PIPE *) dict; + DICT *map; char **cpp; char *dict_type_name; - for (cpp = dict_pipe->map_pipe->argv; (dict_type_name = *cpp) != 0; cpp++) - dict_unregister(dict_type_name); + for (cpp = dict_pipe->map_pipe->argv; (dict_type_name = *cpp) != 0; cpp++) { + if ((map = dict_handle(dict_type_name)) == 0) + msg_panic("%s: dictionary \"%s\" not found", myname, dict_type_name); + dict_close(map); + } argv_free(dict_pipe->map_pipe); vstring_free(dict_pipe->qr_buf); dict_free(dict); @@ -115,8 +120,6 @@ DICT *dict_pipe_open(const char *name, int open_flags, int dict_flags) int match_flags = 0; struct DICT_OWNER aggr_owner; size_t len; - VSTRING *reg_name_buf = vstring_alloc(100); - char *reg_name; /* * Clarity first. Let the optimizer worry about redundant code. @@ -126,8 +129,6 @@ DICT *dict_pipe_open(const char *name, int open_flags, int dict_flags) myfree(saved_name); \ if (argv != 0) \ argv_free(argv); \ - if (reg_name_buf != 0) \ - vstring_free(reg_name_buf); \ return (x); \ } while (0) @@ -176,12 +177,8 @@ DICT *dict_pipe_open(const char *name, int open_flags, int dict_flags) for (cpp = argv->argv; (dict_type_name = *cpp) != 0; cpp++) { if (msg_verbose) msg_info("%s: %s", myname, dict_type_name); - reg_name = dict_make_registered_name(reg_name_buf, dict_type_name, - open_flags, dict_flags); - if ((dict = dict_handle(reg_name)) == 0) - dict = dict_open(dict_type_name, open_flags, dict_flags); - dict_register(reg_name, dict); - argv_replace_one(argv, cpp - argv->argv, reg_name); + dict = dict_open(dict_type_name, open_flags, dict_flags); + argv_replace_one(argv, cpp - argv->argv, dict->reg_name); DICT_OWNER_AGGREGATE_UPDATE(aggr_owner, dict->owner); if (cpp == argv->argv) match_flags = dict->flags & (DICT_FLAG_FIXED | DICT_FLAG_PATTERN); @@ -196,8 +193,7 @@ DICT *dict_pipe_open(const char *name, int open_flags, int dict_flags) dict_pipe->dict.close = dict_pipe_close; dict_pipe->dict.flags = dict_flags | match_flags; dict_pipe->dict.owner = aggr_owner; - dict_pipe->qr_buf = reg_name_buf; - reg_name_buf = 0; + dict_pipe->qr_buf = vstring_alloc(100); dict_pipe->map_pipe = argv; argv = 0; DICT_PIPE_RETURN(&dict_pipe->dict); diff --git a/postfix/src/util/dict_seq.ref b/postfix/src/util/dict_seq.ref index ce7d74432..fad42d90d 100644 --- a/postfix/src/util/dict_seq.ref +++ b/postfix/src/util/dict_seq.ref @@ -1,22 +1,18 @@ +owner=trusted (uid=USER) > put 5 5 -5=5 > put 3 3 -3=3 > put 4 4 -4=4 > put 1 1 -1=1 > put 2 2 -2=2 > first -4=4 -> next 2=2 > next +3=3 +> next +1=1 +> next +4=4 +> next 5=5 > next -3=3 -> next -1=1 -> next not found diff --git a/postfix/src/util/dict_test.c b/postfix/src/util/dict_test.c index ead61b20c..bd2cadd4d 100644 --- a/postfix/src/util/dict_test.c +++ b/postfix/src/util/dict_test.c @@ -86,7 +86,6 @@ void dict_test(int argc, char **argv) dict_allow_surrogate = 1; util_utf8_enable = 1; dict = dict_open(dict_name, open_flags, dict_flags); - dict_register(dict_name, dict); vstream_printf("owner=%s (uid=%ld)\n", dict->owner.status == DICT_OWNER_TRUSTED ? "trusted" : dict->owner.status == DICT_OWNER_UNTRUSTED ? "untrusted" : diff --git a/postfix/src/util/dict_thash.c b/postfix/src/util/dict_thash.c index e95a95739..76b7e9fef 100644 --- a/postfix/src/util/dict_thash.c +++ b/postfix/src/util/dict_thash.c @@ -110,7 +110,7 @@ DICT *dict_thash_open(const char *path, int open_flags, int dict_flags) /* * Reuse the "internal" dictionary type. */ - dict = dict_open3(DICT_TYPE_HT, path, open_flags, dict_flags); + dict = dict_ht_open(path, open_flags, dict_flags); dict_type_override(dict, DICT_TYPE_THASH); /* diff --git a/postfix/src/util/dict_union.c b/postfix/src/util/dict_union.c index bdb367aa3..fc43a0379 100644 --- a/postfix/src/util/dict_union.c +++ b/postfix/src/util/dict_union.c @@ -103,12 +103,17 @@ static const char *dict_union_lookup(DICT *dict, const char *query) static void dict_union_close(DICT *dict) { + static const char myname[] = "dict_union_close"; DICT_UNION *dict_union = (DICT_UNION *) dict; + DICT *map; char **cpp; char *dict_type_name; - for (cpp = dict_union->map_union->argv; (dict_type_name = *cpp) != 0; cpp++) - dict_unregister(dict_type_name); + for (cpp = dict_union->map_union->argv; (dict_type_name = *cpp) != 0; cpp++) { + if ((map = dict_handle(dict_type_name)) == 0) + msg_panic("%s: dictionary \"%s\" not found", myname, dict_type_name); + dict_close(map); + } argv_free(dict_union->map_union); vstring_free(dict_union->re_buf); dict_free(dict); @@ -128,8 +133,6 @@ DICT *dict_union_open(const char *name, int open_flags, int dict_flags) int match_flags = 0; struct DICT_OWNER aggr_owner; size_t len; - VSTRING *reg_name_buf = vstring_alloc(100); - char *reg_name; /* * Clarity first. Let the optimizer worry about redundant code. @@ -139,8 +142,6 @@ DICT *dict_union_open(const char *name, int open_flags, int dict_flags) myfree(saved_name); \ if (argv != 0) \ argv_free(argv); \ - if (reg_name_buf != 0) \ - vstring_free(reg_name_buf); \ return (x); \ } while (0) @@ -189,12 +190,8 @@ DICT *dict_union_open(const char *name, int open_flags, int dict_flags) for (cpp = argv->argv; (dict_type_name = *cpp) != 0; cpp++) { if (msg_verbose) msg_info("%s: %s", myname, dict_type_name); - reg_name = dict_make_registered_name(reg_name_buf, dict_type_name, - open_flags, dict_flags); - if ((dict = dict_handle(reg_name)) == 0) - dict = dict_open(dict_type_name, open_flags, dict_flags); - dict_register(reg_name, dict); - argv_replace_one(argv, cpp - argv->argv, reg_name); + dict = dict_open(dict_type_name, open_flags, dict_flags); + argv_replace_one(argv, cpp - argv->argv, dict->reg_name); DICT_OWNER_AGGREGATE_UPDATE(aggr_owner, dict->owner); if (cpp == argv->argv) match_flags = dict->flags & (DICT_FLAG_FIXED | DICT_FLAG_PATTERN); @@ -209,8 +206,7 @@ DICT *dict_union_open(const char *name, int open_flags, int dict_flags) dict_union->dict.close = dict_union_close; dict_union->dict.flags = dict_flags | match_flags; dict_union->dict.owner = aggr_owner; - dict_union->re_buf = reg_name_buf; - reg_name_buf = 0; + dict_union->re_buf = vstring_alloc(100); dict_union->map_union = argv; argv = 0; DICT_UNION_RETURN(&dict_union->dict); diff --git a/postfix/src/util/dict_utf8_test.ref b/postfix/src/util/dict_utf8_test.ref index 7b825f9e4..225f7c787 100644 --- a/postfix/src/util/dict_utf8_test.ref +++ b/postfix/src/util/dict_utf8_test.ref @@ -1,6 +1,6 @@ owner=trusted (uid=2147483647) > flags -dict flags fixed|lock|replace|utf8_request|utf8_active +dict flags fixed|lock|dup_replace|utf8_request|utf8_active > verbose > get foo foo: not found diff --git a/postfix/src/util/match_list.c b/postfix/src/util/match_list.c index fa533ba65..c4f0cf564 100644 --- a/postfix/src/util/match_list.c +++ b/postfix/src/util/match_list.c @@ -170,12 +170,12 @@ static ARGV *match_list_parse(MATCH_LIST *match_list, ARGV *pat_list, msg_fatal("%s: read file %s: %m", myname, item); } } else if (MATCH_DICTIONARY(item)) { /* type:table */ - vstring_sprintf(buf, "%s%s(%o,%s)", match ? "" : "!", - item, OPEN_FLAGS, dict_flags_str(DICT_FLAGS)); - map_type_name_flags = STR(buf) + (match == 0); + map_type_name_flags = dict_make_registered_name(buf, item, OPEN_FLAGS, + DICT_FLAGS); if (dict_handle(map_type_name_flags) == 0) - dict_register(map_type_name_flags, - dict_open(item, OPEN_FLAGS, DICT_FLAGS)); + dict_open(item, OPEN_FLAGS, DICT_FLAGS); + if (match == 0) + vstring_prepend(buf, "!", 1); argv_add(pat_list, STR(buf), (char *) 0); } else { /* other pattern */ casefold(match_list->fold_buf, match ?