From: Jeremy Harris Date: Fri, 7 Jun 2024 20:45:57 +0000 (+0100) Subject: Hintsdb: tidy coding for DB create X-Git-Tag: exim-4.98-RC1~9 X-Git-Url: https://git.exim.org/exim.git/commitdiff_plain/627391cbcaf3376e201be3ab2815b5abc560ce0c?ds=sidebyside Hintsdb: tidy coding for DB create --- diff --git a/src/src/dbfn.c b/src/src/dbfn.c index 5f30ccf58..f1989d413 100644 --- a/src/src/dbfn.c +++ b/src/src/dbfn.c @@ -67,8 +67,6 @@ Arguments: name The single-component name of one of Exim's database files. flags Either O_RDONLY or O_RDWR, indicating the type of open required; O_RDWR implies "create if necessary" -XXX this is a mess. hintsdb.h has grown lots of code expecting O_CREAT -XXX with the obvious semantics, and not that described above. dbblock Points to an open_db block to be filled in. lof If TRUE, write to the log for actual open failures (locking failures are always logged). @@ -79,17 +77,14 @@ Returns: NULL if the open failed, or the locking failed. After locking On success, dbblock is returned. This contains the dbm pointer and the fd of the locked lock file. - -There are some calls that use O_RDWR|O_CREAT for the flags. Having discovered -this in December 2005, I'm not sure if this is correct or not, but for the -moment I haven't changed them. */ open_db * -dbfn_open(uschar *name, int flags, open_db *dbblock, BOOL lof, BOOL panic) +dbfn_open(const uschar * name, int flags, open_db * dbblock, + BOOL lof, BOOL panic) { int rc, save_errno; -BOOL read_only = flags == O_RDONLY; +BOOL read_only = flags & O_RDONLY; flock_t lock_data; uschar dirname[PATHLEN], filename[PATHLEN]; @@ -165,6 +160,7 @@ databases - often this is caused by non-matching db.h and the library. To make it easy to pin this down, there are now debug statements on either side of the open call. */ +flags &= O_RDONLY | O_RDWR; snprintf(CS filename, sizeof(filename), "%s/%s", dirname, name); priv_drop_temp(exim_uid, exim_gid); @@ -202,7 +198,6 @@ DEBUG(D_hints_lookup) debug_printf_indent("opened hints database %s: flags=%s\n", filename, flags == O_RDONLY ? "O_RDONLY" : flags == O_RDWR ? "O_RDWR" - : flags == (O_RDWR|O_CREAT) ? "O_RDWR|O_CREAT" : "??"); /* Pass back the block containing the opened database handle and the open fd diff --git a/src/src/dbfunctions.h b/src/src/dbfunctions.h index 1f0dec1f7..0b0bcab22 100644 --- a/src/src/dbfunctions.h +++ b/src/src/dbfunctions.h @@ -14,7 +14,7 @@ void dbfn_close(open_db *); int dbfn_delete(open_db *, const uschar *); -open_db *dbfn_open(uschar *, int, open_db *, BOOL, BOOL); +open_db *dbfn_open(const uschar *, int, open_db *, BOOL, BOOL); void *dbfn_read_with_length(open_db *, const uschar *, int *); void *dbfn_read_enforce_length(open_db *, const uschar *, size_t); uschar *dbfn_scan(open_db *, BOOL, EXIM_CURSOR **); diff --git a/src/src/directory.c b/src/src/directory.c index 94303db0b..876d097b5 100644 --- a/src/src/directory.c +++ b/src/src/directory.c @@ -36,7 +36,7 @@ Returns: panic on failure if panic is set; otherwise return FALSE; */ BOOL -directory_make(const uschar *parent, const uschar *name, +directory_make(const uschar * parent, const uschar * name, int mode, BOOL panic) { BOOL use_chown = parent == spool_directory && geteuid() == root_uid; diff --git a/src/src/exim_dbutil.c b/src/src/exim_dbutil.c index 4e8b291c6..b152df145 100644 --- a/src/src/exim_dbutil.c +++ b/src/src/exim_dbutil.c @@ -287,28 +287,23 @@ Returns: NULL if the open failed, or the locking failed. */ open_db * -dbfn_open(uschar *name, int flags, open_db *dbblock, BOOL lof, BOOL panic) +dbfn_open(const uschar * name, int flags, open_db * dbblock, + BOOL lof, BOOL panic) { int rc; struct flock lock_data; -BOOL read_only = flags == O_RDONLY; +BOOL read_only = flags & O_RDONLY; uschar * dirname, * filename; /* The first thing to do is to open a separate file on which to lock. This ensures that Exim has exclusive use of the database before it even tries to open it. If there is a database, there should be a lock file in existence. */ -#ifdef COMPILE_UTILITY if ( asprintf(CSS &dirname, "%s/db", spool_directory) < 0 || asprintf(CSS &filename, "%s/%s.lockfile", dirname, name) < 0) return NULL; -#else -dirname = string_sprintf("%s/db", spool_directory); -filename = string_sprintf("%s/%s.lockfile", dirname, name); -#endif -dbblock->lockfd = Uopen(filename, flags, 0); -if (dbblock->lockfd < 0) +if ((dbblock->lockfd = Uopen(filename, O_RDWR|O_CREAT, 0)) < 0) { printf("** Failed to open database lock file %s: %s\n", filename, strerror(errno)); @@ -331,7 +326,7 @@ if (sigalrm_seen) errno = ETIMEDOUT; if (rc < 0) { printf("** Failed to get %s lock for %s: %s", - flags & O_WRONLY ? "write" : "read", + read_only ? "read" : "write", filename, errno == ETIMEDOUT ? "timed out" : strerror(errno)); (void)close(dbblock->lockfd); @@ -341,14 +336,11 @@ if (rc < 0) /* At this point we have an opened and locked separate lock file, that is, exclusive access to the database, so we can go ahead and open it. */ -#ifdef COMPILE_UTILITY if (asprintf(CSS &filename, "%s/%s", dirname, name) < 0) return NULL; -#else -filename = string_sprintf("%s/%s", dirname, name); -#endif -dbblock->dbptr = exim_dbopen(filename, dirname, flags, 0); -if (!dbblock->dbptr) +if (flags & O_RDWR) flags |= O_CREAT; + +if (!(dbblock->dbptr = exim_dbopen(filename, dirname, flags, 0))) { printf("** Failed to open DBM file %s for %s:\n %s%s\n", filename, read_only? "reading" : "writing", strerror(errno), @@ -1427,3 +1419,5 @@ return 0; #endif /* EXIM_TIDYDB */ /* End of exim_dbutil.c */ +/* vi: aw ai sw=2 +*/ diff --git a/src/src/hintsdb.h b/src/src/hintsdb.h index 17b5c243c..3898b0221 100644 --- a/src/src/hintsdb.h +++ b/src/src/hintsdb.h @@ -23,7 +23,7 @@ binary blobs. The API is: Functions: - exim_dbopen + exim_dbopen O_RDONLY/O_RDWR, optionally OR'd with O_CREAT exim_dbclose exim_dbget exim_dbput @@ -542,8 +542,10 @@ if (db_create(&b, dbp, 0) == 0) { dbp->app_private = b; if (b->open(b, NULL, CS name, NULL, - flags == O_RDONLY ? DB_UNKNOWN : DB_HASH, - flags == O_RDONLY ? DB_RDONLY : DB_CREATE, + flags & O_CREAT ? DB_HASH : DB_UNKNOWN, + flags & O_CREAT ? DB_CREATE + : flags & O_RDONLY ? DB_RDONLY + : 0, /*XXX is there a writeable if exists option? */ mode) == 0 ) return dbp; @@ -676,8 +678,10 @@ EXIM_DB * dbp; return db_create(&dbp, NULL, 0) == 0 && ( dbp->set_errcall(dbp, dbfn_bdb_error_callback), dbp->open(dbp, CS name, NULL, - flags == O_RDONLY ? DB_UNKNOWN : DB_HASH, - flags == O_RDONLY ? DB_RDONLY : DB_CREATE, + flags & O_CREAT ? DB_HASH : DB_UNKNOWN, + flags & O_CREAT ? DB_CREATE + : flags & O_RDONLY ? DB_RDONLY + : 0, /*XXX*/ mode) ) == 0 ? dbp : NULL; diff --git a/src/src/verify.c b/src/src/verify.c index c5b99a97c..b1e5d6802 100644 --- a/src/src/verify.c +++ b/src/src/verify.c @@ -294,7 +294,7 @@ implying some kind of I/O error. We don't want to write the cache in that case. Otherwise the value is ccache_accept, ccache_reject, or ccache_reject_mfnull. */ if (dom_rec->result != ccache_unknown) - if (!(dbm_file = dbfn_open(US"callout", O_RDWR|O_CREAT, &dbblock, FALSE, TRUE))) + if (!(dbm_file = dbfn_open(US"callout", O_RDWR, &dbblock, FALSE, TRUE))) { HDEBUG(D_verify) debug_printf_indent("callout cache: not available\n"); } @@ -316,7 +316,7 @@ is disabled. */ if (done && addr_rec->result != ccache_unknown) { if (!dbm_file) - dbm_file = dbfn_open(US"callout", O_RDWR|O_CREAT, &dbblock, FALSE, TRUE); + dbm_file = dbfn_open(US"callout", O_RDWR, &dbblock, FALSE, TRUE); if (!dbm_file) { HDEBUG(D_verify) debug_printf_indent("no callout cache available\n"); @@ -3523,7 +3523,7 @@ dbdata_callout_cache_address cache_address_record; if (!pos_cache && !neg_cache) return; -if (!(dbm_file = dbfn_open(US"callout", O_RDWR|O_CREAT, &dbblock, FALSE, TRUE))) +if (!(dbm_file = dbfn_open(US"callout", O_RDWR, &dbblock, FALSE, TRUE))) { HDEBUG(D_verify) debug_printf_indent("quota cache: not available\n"); return;